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

Avoid mangling /bin non-perl shebangs on merged-/usr systems #330

Merged
merged 1 commit into from
Nov 15, 2018

Conversation

ilmari
Copy link
Contributor

@ilmari ilmari commented Nov 13, 2018

If the shebang is absolute and exists in PATH, but was not the first
one found, leave it alone if it's actually the same file as first one.

This avoids packages built on merged-/usr systems with /usr/bin before
/bin in the path breaking when installed on systems without merged
/usr. See e.g. https://bugs.debian.org/913637.

@mohawk2
Copy link
Member

mohawk2 commented Nov 13, 2018

It seems like a good idea to have tests for this?

@Grinnz
Copy link
Contributor

Grinnz commented Nov 13, 2018

This seems awfully special-cased... That said I'm not sure the purpose of mangling non-perl shebangs to begin with...

@ilmari
Copy link
Contributor Author

ilmari commented Nov 13, 2018

@mohawk2 the whole PATH-related bit of shebang rewriting is currently mostly untested, other than to check that it doesn't rewrite a non-existing non-perl interpreter.

If the shebang is absolute and exists in PATH, but was not the first
one found, leave it alone if it's actually the same file as first one.

This avoids packages built on merged-/usr systems with /usr/bin before
/bin in the path breaking when installed on systems without merged
/usr.  See e.g. https://bugs.debian.org/913637.
@ilmari
Copy link
Contributor Author

ilmari commented Nov 14, 2018

@mohawk2 tests added

@mohawk2
Copy link
Member

mohawk2 commented Nov 15, 2018

Seen. They look great.

@bingos bingos merged commit 9766f9c into Perl-Toolchain-Gang:master Nov 15, 2018
@ilmari ilmari deleted the shebang-usr-merge branch November 16, 2018 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants