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.

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

itstool: fix double-shebang issue on macOS #74942

Merged
merged 2 commits into from Apr 3, 2020

Conversation

burke
Copy link
Contributor

@burke burke commented Dec 3, 2019

I haven't tried this on Linux, where it should have no effect, but it does make make libgda (which is failing on darwin) work correctly on my machine.

@FRidh
Copy link
Member

FRidh commented Dec 4, 2019

We need a generic solution for this and should not be fixing individual packages. cc @NixOS/darwin-maintainers any ideas? Maybe a darwin-specific hook that patches all shebangs to prefix usr/bin/env?

@lavoiesl
Copy link
Contributor

lavoiesl commented Dec 4, 2019

Would fix #73762

@FRidh FRidh added this to Needs review in Staging Dec 15, 2019
# wrapper, which is perfectly happy to execute an interpreted script.
#
# However, we don't want to do this on Linux, which only allows one argument
# in a shebang.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like something we can update in patch-shebangs.sh. I didn't realize that /usr/bin/env worked differently! For instance, skipping it here: https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/setup-hooks/patch-shebangs.sh#L64

@jtojnar jtojnar mentioned this pull request Jan 17, 2020
10 tasks
@vcunat
Copy link
Member

vcunat commented Feb 28, 2020

I think this is what is blocking the nixpkgs-20.03-darwin channel: darwin-tested -> gimp -> shared-mime-info. /cc @NixOS/nixos-release-managers.

@vcunat vcunat added 1.severity: channel blocker Blocks a channel 9.needs: port to stable A PR needs a backport to the stable release. labels Feb 28, 2020
@jtojnar
Copy link
Contributor

jtojnar commented Mar 17, 2020

Would just patching newInterpreterLine on Darwin to @coreutils-or-wherever-env-is-on-mac@/bin/env -S $newPath $args work? -S should split the arguments according to man page in this answer so it should work even with multiple arguments. cc @NixOS/darwin-maintainers

Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a better fix for this using wrapPython:

Please cherry-pick this commit instead: fa247de4d70015608f8f70a8192757da6595488f

@LnL7
Copy link
Member

LnL7 commented Mar 31, 2020

@Mic92 That looks like the way to go to me.

While I guess changing patchShebangs would work around this I'd much rather avoid double wrapping in general, it's rather ugly IMHO.

@burke
Copy link
Contributor Author

burke commented Mar 31, 2020

Cherry-pick completed. Thanks!

@domenkozar
Copy link
Member

@GrahamcOfBorg build itstool

@domenkozar
Copy link
Member

Should this go to staging?

@worldofpeace
Copy link
Contributor

Should this go to staging?

Yes, it appears to rebuild the world (not just on darwin). I think we can stand a mass rebuild just on darwin though.

@grahamc grahamc added this to the 20.03 milestone Apr 3, 2020
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/go-no-go-meeting-nixos-20-03-markhor/6495/16

@FRidh FRidh changed the base branch from master to staging April 3, 2020 20:04
@FRidh FRidh merged commit 2bec129 into NixOS:staging Apr 3, 2020
Staging automation moved this from Needs review to Done Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Staging
  
Done
Development

Successfully merging this pull request may close these issues.

None yet