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

Change shebang to /usr/bin/env/perl #847

Merged
merged 2 commits into from Jul 13, 2017

Conversation

@tkw1536
Copy link
Contributor

tkw1536 commented Jul 8, 2017

On certain machines, when run directly, all scripts would default to use
the perl interpreter located in /usr/bin. This can cause problems on
machines where perl is located in a different path, or where multiple
perl instances are present on a system (such as on Mac when using
homebrew).

This PR updates the shebang, the line which starting with #!
indicating the interpreter to use when running a file, from
/usr/bin/perl to /usr/bin/env perl. This makes sure not to hard-code the
perl path to /usr/bin, but to instead search $PATH for the appropriate
file.

On certain machines, when run directly, all scripts would default to use
the perl interpreter located in /usr/bin. This can cause problems on
machines where perl is located in a different path, or where multiple
perl instances are present on a system (such as on Mac when using
homebrew).

This commit updates the shebang, the line which starting with #!
indicating the interpreter to use when running a file, from
/usr/bin/perl to /usr/bin/env perl. This makes sure not to hard-code the
perl path to /usr/bin, but to instead search $PATH for the appropriate
file.
@brucemiller

This comment has been minimized.

Copy link
Owner

brucemiller commented Jul 8, 2017

@dginev

This comment has been minimized.

Copy link
Collaborator

dginev commented Jul 8, 2017

Tom's change looks to be the best practice:
http://perldoc.perl.org/perlrun.html

But why did travis fail?

@dginev

This comment has been minimized.

Copy link
Collaborator

dginev commented Jul 8, 2017

Travis failed because the env form does not allow passing arguments, such as -w to the interpreter. And there are trade-offs it seems:
https://unix.stackexchange.com/a/29620

@dginev

This comment has been minimized.

Copy link
Collaborator

dginev commented Jul 8, 2017

As can be seen in the wisdom in the unix stackexchange comment i shared, the -w can be upgraded to an explicit use warnings; in each of the executables, which solves the travis failure. Personally I am happy either way, I have used both forms in the past and my setups are simple ("standard"?) enough to work with either form.

@tkw1536

This comment has been minimized.

Copy link
Contributor Author

tkw1536 commented Jul 8, 2017

It turns out that some /usr/bin/env implementations do not support
passing parameters to the interporeter. This caused problems, in
particular causing the Travis Tests to fail.

After a suggestion from @dginev, this commit fixes the issue by removing
the -w flag from all shebang lines and instead adding a 'use warnings;'
where neccessary.
@brucemiller brucemiller merged commit 8801a10 into brucemiller:master Jul 13, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tkw1536 tkw1536 deleted the tkw1536:shebang-fix branch Jul 13, 2017
@brucemiller

This comment has been minimized.

Copy link
Owner

brucemiller commented Aug 4, 2017

For the record, there is a slight downside to this: If you run valgrind on latexml, it no longer "sees" perl, but (apparently) checks the memory usage of env(?). The easy workaround is

valgrind perl path/to/latexml ...

Just so you know...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.