Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

zrythm: init at 0.8.252 #85553

Closed
wants to merge 4 commits into from
Closed

zrythm: init at 0.8.252 #85553

wants to merge 4 commits into from

Conversation

magnetophon
Copy link
Member

@magnetophon magnetophon commented Apr 19, 2020 •
edited

Problem description:

When I try to install this with: nix-env -f $NIXPKGS -iA zrythm I get:

resources/meson.build:30:0: ERROR: Could not execute command "/usr/bin/env sh /tmp/nix-build-zrythm-0.8.252.drv-0/source/build/resources/gen_gtk_resources_xml_wrap.sh /tmp/nix-build-zrythm-0.8.252.drv-0/source/resources /tmp/nix-build-zrythm-0.8.252.drv-0/source/build/resources/zrythm.gresources.xml".

When I do:

nix-shell $NIXPKGS -A zrythm
mkdir zr
cd zr
unpackPhase
genericBuild

it builds.

I can then successfully run it with /nix/store/nkyh59931zydxb80j3j5i1whcva6ijbl-zrythm-0.8.252/bin/zrythm_launch

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@magnetophon magnetophon mentioned this pull request Apr 23, 2020
Open
@jtojnar
Copy link
Contributor

jtojnar commented Apr 23, 2020

/usr/bin/env is not available in sandbox so you need to run patchShebangs resources/gen_gtk_resources_xml_wrap.sh in postPatch.

jtojnar
jtojnar reviewed Apr 23, 2020
View reviewed changes
pkgs/development/libraries/audec/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/audec/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libcyaml/default.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/zrythm/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/audec/default.nix Outdated Show resolved Hide resolved
@magnetophon
Copy link
Member Author

@jtojnar Thanks for the feedback, all done!
Any idea what causes patchShebangs to not work?
It outputs that it's working, but when I cat the file directly after, it's unchanged.

Then we only need a solution for the dconf issue and this pkg is good to go!

@jtojnar
Copy link
Contributor

jtojnar commented Apr 24, 2020

patchShebangs only works on executable files so you might need to chmod +x it before.

Also it should be done in postPatch to avoid conflicts in case patches will ever affect the file.

jtojnar
jtojnar reviewed Jun 12, 2020
View reviewed changes
pkgs/applications/audio/zrythm/default.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/zrythm/default.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/zrythm/default.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/zrythm/default.nix Outdated Show resolved Hide resolved
@magnetophon
Copy link
Member Author

magnetophon commented Jun 12, 2020 •
edited

@jtojnar Thanks for the review.
All done!

@0x4A6F
Copy link
Member

0x4A6F commented Jul 25, 2020

@magnetophon Is this PR still WIP as concluded from the title?

@magnetophon
Copy link
Member Author

I think it's good now, apart from the meanwhile outdated version.

@0x4A6F
Copy link
Member

0x4A6F commented Jul 25, 2020

@magnetophon Please drop WIP from the title and ping a reviewer again. This PR might have been overlooked due to WIP in the title.
And thanks for your contribution and effort.

@magnetophon magnetophon changed the title WIP: zrythm: init at 0.8.252 zrythm: init at 0.8.252 Jul 25, 2020
@magnetophon
Copy link
Member Author

@0x4A6F Thanks.
I changed the title, but I'd rather not ping anyone on this.
Zrythm itself is in alpha, and I have many more PR's that would be more useful in nixpkgs.
It would be great if you could have a look!

@magnetophon magnetophon mentioned this pull request Oct 3, 2020
Closed
@zyansheep
Copy link
Contributor

any update on this?

@Narice
Copy link
Contributor

Narice commented Jun 23, 2021

It would be great to merge this PR @magnetophon and @jtojnar.
Even though the software in itself is in alpha, it is still of interest to some people in the community and the expression in itself is complete and works.

@SuperSandro2000
Copy link
Member

It would be great to merge this PR @magnetophon and @jtojnar.

It cannot be merged while it has merge conflicts.

@Narice
Copy link
Contributor

Narice commented Jun 24, 2021

Is there a way for me to help directly on this PR or is it necessary for example to make another PR if I want to help fixing the merge conflicts @SuperSandro2000?

@SuperSandro2000
Copy link
Member

Is there a way for me to help directly on this PR or is it necessary for example to make another PR if I want to help fixing the merge conflicts @SuperSandro2000?

You could do that if you like.

@felix-andreas
Copy link
Member

I resolved the conflicts and updated the zrythm version on felix-andreas@38185e8

Maybe this can be merged into this PR or should I open a new PR?

@SuperSandro2000
Copy link
Member

I resolved the conflicts and updated the zrythm version on andreasfelix@38185e8

Maybe this can be merged into this PR or should I open a new PR?

Please open a new PR. That is easier and makes the git history cleaner.

@Narice
Copy link
Contributor

Narice commented Jul 5, 2021

@andreasfelix would it be possible for you to make a PR for your branch, it looks really awesome!
Also, thanks for the work, the changes you did doesn't seem easy

@felix-andreas felix-andreas mentioned this pull request Jul 6, 2021
Closed
11 tasks
@tshaynik
Copy link
Contributor

tshaynik commented Nov 9, 2021

This PR can be closed now that #144547 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar left review comments

Assignees
No one assigned
Labels
2.status: merge conflict 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@magnetophon @jtojnar @0x4A6F @zyansheep @Narice @SuperSandro2000 @felix-andreas @tshaynik @ryantm