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

perlPackages.GitAutoFixup: fix too long shebang under darwin #116799

Merged
merged 1 commit into from Mar 20, 2021
Merged

perlPackages.GitAutoFixup: fix too long shebang under darwin #116799

merged 1 commit into from Mar 20, 2021

Conversation

aisamu
Copy link
Contributor

@aisamu aisamu commented Mar 18, 2021

Motivation for this change

Similarly to #91419 and other, this package had a shebang line longer than 512 characters, making the invocation fail on OS X with an exec format error:

zsh: /Users/aisamu/.nix-profile/bin/git-autofixup: bad interpreter: /nix/store/f18m26x5d7xywdx12ni631j79d5k1kjw-perl-5.32.0/bin/perl -I/nix/store/f18m26x5d7xywdx12ni631j79d5k1kjw-perl-5.32.0/lib: exec format error
Things done
  • Built on platform(s)
    • macOS
  • Tested execution of all binary files (usually in ./result/bin/)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@aisamu aisamu requested a review from stigtsp as a code owner March 18, 2021 23:25
@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Mar 18, 2021
@r-rmcgibbo
Copy link

r-rmcgibbo commented Mar 18, 2021

Result of nixpkgs-review pr 116799 at 84addff6 run on aarch64-linux 1

2 packages built successfully:

Result of nixpkgs-review pr 116799 at 84addff6 run on x86_64-linux 1

2 packages built successfully:

Comment on lines 8685 to 8686
nativeBuildInputs = pkgs.lib.optional pkgs.stdenv.isDarwin pkgs.shortenPerlShebang;
postInstall = pkgs.lib.optionalString pkgs.stdenv.isDarwin ''
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nativeBuildInputs = pkgs.lib.optional pkgs.stdenv.isDarwin pkgs.shortenPerlShebang;
postInstall = pkgs.lib.optionalString pkgs.stdenv.isDarwin ''
nativeBuildInputs = lib.optional stdenv.isDarwin pkgs.shortenPerlShebang;
postInstall = lib.optionalString stdenv.isDarwin ''

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oopsie, thanks!

Copy link
Contributor Author

@aisamu aisamu Mar 19, 2021

Choose a reason for hiding this comment

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

Would it be OK to fixup! this? (and squash+force-push it, that is)

Copy link
Member

@stigtsp stigtsp Mar 19, 2021

Choose a reason for hiding this comment

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

Yes pls (as long as it results in one commit matching CONTRIBUTING.md). Also, it would be great if you could change the attribute name in the commit message from perl-packages.GitAutoFixup to perlPackages.GitAutoFixup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done and done!

change perl-packages.GitAutoFixup to perlPackages.GitAutoFixup

I was a bit confused about what to name it, since we use it as (e.g.) perl532Packages.***, but this would also change the the other perl versions as well (e.g. perl530Packages.*** )!

Does the snake-case -> camelCase pattern apply to all top-level packages?

Copy link
Member

@stigtsp stigtsp Mar 20, 2021

Choose a reason for hiding this comment

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

Thx for the fixes.

perlPackages always points to the latest maint version of perl, like perl532Packages. I'm not sure there is a snake-case to camelCase rule tbh.

@stigtsp
Copy link
Member

stigtsp commented Mar 20, 2021

@SuperSandro2000 Can you test on darwin pls? My darwin machine is unavailable today :-/

@stigtsp stigtsp changed the title perl-packages.GitAutoFixup: fix too long shebang under darwin perlPackages.GitAutoFixup: fix too long shebang under darwin Mar 20, 2021
@SuperSandro2000
Copy link
Member

@SuperSandro2000 Can you test on darwin pls? My darwin machine is unavailable today :-/

Tested and works fine.

Copy link
Member

@stigtsp stigtsp left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for the contribution, and welcome to the NixOS project! :-)

@stigtsp stigtsp merged commit 739df23 into NixOS:master Mar 20, 2021
@aisamu
Copy link
Contributor Author

aisamu commented Mar 22, 2021

Thank you for all the hand-holding!
It's the least I could do after all the joy nix has brought me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants