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

Use #!/usr/bin/env perl instead of #!/usr/bin/perl to be able to use non standard perl in environment PATH. #51

Closed
mjscosta opened this issue Dec 12, 2018 · 7 comments

Comments

@mjscosta
Copy link

Hi, I need to use lcov in a non standard environment where perl interpreter is determined by the environment, and thus the current usage of #!/usr/bin/perl prevents it from working in such environment.

I've prepared a patch with the change, please confirm if it is reasonable and accepted.

Thanks.

@mjscosta mjscosta changed the title User #!/usr/bin/env perl instead of #!/usr/bin/perl to be able to use non standard perl in environment PATH. Use #!/usr/bin/env perl instead of #!/usr/bin/perl to be able to use non standard perl in environment PATH. Dec 12, 2018
@latk
Copy link

latk commented Dec 12, 2018

I'm not sure whether either alternative is always better. Choosing #!/usr/bin/env perl is inappropriate if lcov was installed for the system perl. I've encountered related problems in other Perl applications where required modules would no longer be found when the application was executed in a different perl's context. Here, this doesn't quite apply because lcov is implemented as a collection of self-contained scripts rather than a module, but it highlights potential difficulties.

Is there a way to fix up the shebang during installation so that lcov will always run with the perl it was installed under? E.g. I think Perl's EUMM is able to perform such rewriting (doc link, discussion), but it's use may be inappropriate in the context of lcov.

A better solution might be to have the make install call fix up the shebang line on the fly if some variable is set.

@mjscosta
Copy link
Author

Hi, indeed I was not aware of this issue. I've came across it packaging lcov for anaconda, and since they use contained environments, perl is in a non standard location. I've patched lcov scripts for that package, and it's working.

I'll consider patching the Makefile according to your suggestion.

Thanks for the feedback!

@oberpar
Copy link
Contributor

oberpar commented Dec 18, 2018

I can see the value of using env to locate the interpreter in the non-installed version of LCOV. There are some platforms though where the use of env as interpreter for system-installed scripts is discouraged [1]. While this may be automatically 'fixed' while building packages (as in Fedora 28), I would prefer if LCOV provided the same functionality independently of the platform it is being used on.

I like the idea proposed by @latk. I could imagine the following set of patches:

  1. Change instances of perl -w to use warning
  2. Use /usr/bin/env to locate interpreter scripts, both in Perl and Bash scripts
  3. Modify the install.sh script to convert env invocations to absolute paths. The target paths could be taken from environment variables that are pre-defined in the LCOV Makefile to their current values (/usr/bin/perl, /bin/bash), but which could also be overridden during package installation, e.g. using
make install PERL_PATH=/usr/bin/perl BASH_PATH=/bin/bash

[1] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shebang_lines

@oberpar
Copy link
Contributor

oberpar commented Jan 10, 2019

Is anyone currently working on implementing this change? If yes, and there is a chance that this is finished by start of February, it could be included in the next LCOV release.

@mjscosta
Copy link
Author

mjscosta commented Jan 10, 2019

Hi @oberpar , the solution you described above is not clear to me.

The for the use case I need the location of the interpreter is located in the PATH, for which it cannot be determined at install time. The concrete use case is supporting lcov in anaconda environments.

So the make install for this specific use-case would me some flag, e.g.:

make install USE_ENV_INTERPERTER=1

That would replace the shebang line by #!/usr/bin/env perl same same for bash #!/usr/bin/env bash.
Let me know if you agree with these

@oberpar
Copy link
Contributor

oberpar commented Jan 11, 2019

I can see three different use cases that should be addressed:

  1. Run lcov without installation (shebang line #!/usr/bin/env perl)
  2. Install lcov with fixed interpreter path (shebang line #!/usr/bin/perl). This should be the default for make install.
  3. Install lcov with variable interpreter path (shebang line #!/usr/bin/env perl). This should be somehow selectable when running make install.

This could be achieved as follows:

  1. All interpreter lines in the git source repository should be replaced by #!/usr/bin/env equivalents.
  2. During make install, if PERL_PATH is set, the shebang lines of installed Perl programs should be replaced with the value of #!$PERL_PATH.
  3. The lcov Makefile should contain a default value for PERL_PATH (pointing to /usr/bin/perl).

For the use-case you described, you would unset PERL_PATH during installation:

make install PERL_PATH=

Note: BASH_PATH is not required as no installed script is using Bash.

@oberpar
Copy link
Contributor

oberpar commented Feb 28, 2019

This is now implemented with commits 2ff99ae, 0b378cb, and 74bae96.

Thanks to everyone involved for their suggestions!

@oberpar oberpar closed this as completed Feb 28, 2019
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

No branches or pull requests

3 participants