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

Inconsistant paths in Perl script shabang lines. #141

Open
jame opened this issue Mar 24, 2013 · 25 comments
Open

Inconsistant paths in Perl script shabang lines. #141

jame opened this issue Mar 24, 2013 · 25 comments

Comments

@jame
Copy link
Contributor

jame commented Mar 24, 2013

There are multiple Perl scripts in the MisterHouse distribution but when a shabang line is actually in the file, there are multiple locations that it might point to. Examples are as follows:

  • /bin/perl
  • /local/bin/perl
  • /usr/local/bin/perl
  • d:/perl/bin/perl.exe
  • /usr/bin/perl
@marcmerlin
Copy link
Collaborator

On Sun, Mar 24, 2013 at 12:16:07PM -0700, Robert James Clay wrote:

There are multiple Perl scripts in the MisterHouse distribution but when a shabang line is actually in the file, there are multiple locations that it might point to. Examples are as follows:

  • /bin/perl
  • /local/bin/perl
  • /usr/local/bin/perl
  • d:/perl/bin/perl.exe
  • /usr/bin/perl

#!/usr/bin/perl should be the standard IMO

Marc

"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
.... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/

@hollie
Copy link
Owner

hollie commented Mar 24, 2013

Hi Marc,

Op 24-mrt.-2013, om 20:17 heeft Marc MERLIN [email protected] het volgende geschreven:

#!/usr/bin/perl should be the standard IMO

would
#!/usr/bin/env perl

not be more portable?

http://en.wikipedia.org/wiki/Shebang_(Unix)#Portability

Kind regards,
Lieven.

@marcmerlin
Copy link
Collaborator

On Sun, Mar 24, 2013 at 12:19:56PM -0700, Lieven Hollevoet wrote:

Hi Marc,

Op 24-mrt.-2013, om 20:17 heeft Marc MERLIN [email protected] het volgende geschreven:

#!/usr/bin/perl should be the standard IMO

would
#!/usr/bin/env perl

not be more portable?

It would, if there is anyone running any unix where perl isn't
/usr/bin/perl, I would be ok with that instead.

But honestly, it's been a very long time since I've seen a unix system (more
specifically linux) where perl wasn't /usr/bin/perl.

Either work for me though.

Marc

"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
.... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/

@hollie
Copy link
Owner

hollie commented Mar 24, 2013

Op 24-mrt.-2013, om 20:23 heeft Marc MERLIN [email protected] het volgende geschreven:

[..]

would
#!/usr/bin/env perl

not be more portable?

It would, if there is anyone running any unix where perl isn't
/usr/bin/perl, I would be ok with that instead.

But honestly, it's been a very long time since I've seen a unix system (more
specifically linux) where perl wasn't /usr/bin/perl.

Yes, /usr/bin/perl if you're running the system-provided perl. I'm a big fan of perlbrew that allows you to test with various versions of perl. Result:

nessie:hue-perl lieven$ which perl
/Users/lieven/perl5/perlbrew/perls/perl-5.16.0/bin/perl

So yes, other paths to perl do exist :-)

Regards,
Lieven.

@marcmerlin
Copy link
Collaborator

On Sun, Mar 24, 2013 at 12:27:23PM -0700, Lieven Hollevoet wrote:

Yes, /usr/bin/perl if you're running the system-provided perl. I'm a big fan of perlbrew that allows you to test with various versions of perl. Result:

nessie:hue-perl lieven$ which perl
/Users/lieven/perl5/perlbrew/perls/perl-5.16.0/bin/perl

So yes, other paths to perl do exist :-)

Fair enough /usr/bin/env perl if you'd like is fine with me :)

Marc

"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
.... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/

@jame
Copy link
Contributor Author

jame commented Mar 24, 2013

On Debian, the path "/usr/bin/perl" is standard and is required by policy. Therefore, QA tools like Lintian will come up with either errors or warnings if it finds something different (depending on how it's different). So far, It's found 12 files with the issue, most of which it considered as errors. I've fixed those by setting the path to the standard one for Debian (see pull request 142). I also fixed another file in the same way, which didn't have the line at all; reference issue # 139 for that...

@jame
Copy link
Contributor Author

jame commented Apr 18, 2013

It can't be used in the Debian packaging but if the consensus is to use 'usr/bin/env perl' as the standard shabang path in MisterHouse, I could update the fixes I did to that so that the pull request for those files could be pulled in.

@hollie
Copy link
Owner

hollie commented Apr 18, 2013

Hey RJ,

I'm certainly no authority on this subject, I was just explaining my method of working. Apparently few other people have an opinion on this subject or they haven't noticed this discussion. Feel free to pick whatever suits you best.

Lieven.

Op 18-apr.-2013, om 22:41 heeft Robert James Clay [email protected] het volgende geschreven:

It can't be used in the Debian packaging but if the consensus is to use 'usr/bin/env perl' as the standard shabang path in MisterHouse, I could update the fixes I did to that so that the pull request for those files could be pulled in.


Reply to this email directly or view it on GitHub.

@krkeegan
Copy link
Collaborator

krkeegan commented Jun 1, 2013

Merge #209 fixed a bunch of shebangs. Was that all of them? Is this still an open issue?

@jame
Copy link
Contributor Author

jame commented Jun 2, 2013

On Fri, 31 May 2013 19:35:32 -0700
Kevin Robert Keegan [email protected] wrote:

Merge #209 fixed a bunch of shebangs. Was that all of them?

All that I found then... I'll admit I haven't reviewed every script
in the distribution but I will note that I'm not currently getting the
'wrong-path-for-interpreter' type errors on a test package build on the
code as of 31 May. (OTOH, it's also true that if the file isn't marked
as an executable, it's not even checked for a shabang line...)

Is this still an open issue?

That depends... I think there should be some kind of project
policy regarding what's used in the shabang line in Perl scripts
(whatever that might turn out to be), so that it's consistent across all
of the scripts in MisterHouse. That does not mean that this issue
cannot be closed now, although it could still be used to track the
creation of such a policy and closed when that has been apropriately
published.

Robert James Clay
[email protected]
[email protected]

@hollie
Copy link
Owner

hollie commented Aug 18, 2013

Hey guys,

I guess we need to discuss this further. I hit a problem with the Perl shebangs in the scripts under the 'bin' folder when I try to run MisterHouse on a custom installed Perl version on OS X.

E.g. bin/get_email hardcodes the Perl executable to /usr/bin/perl which causes the module search path to go bezerk. For more details see #247

@krkeegan
Copy link
Collaborator

I wanted to chime in and say I think this is an important issue to resolve. However, I am not a perl maven so I am not sure I understand all of the ramifications for something like this. @hollie's solution seems reasonable. Good-old perlmonks has some decent posts on the subject that suggest that this may be acceptable. Didn't this originally arise in the porting to debian? Is this allowed in debian packages?

@hollie
Copy link
Owner

hollie commented Aug 26, 2013

After reading some more on this: apparently the opinions on hardcoding either /usr/bin/perl or /usr/bin/env perl in scripts are very mixed.

The most sane/safe advice I found was that the installer should find out what perl to use and should replace the shebang line with the correct content when installing the script(s). However, since MisterHouse has no installer this is currently not really on option to go for.

I think the first mention of this shebang line was indeed when @jame was cleaning out the various shebangs we had for various scripts when packaging for debian, but since the current shebang causes parts of MisterHouse to fail on non-standard installs (i.e. systems that don't use the system-provided Perl) I raised it again.

I'm unsure how to proceed but I agree with @krkeegan we need to make a decision on this issue. I'm in favor of /usr/bin/env perl as it allows the user to select what perl to run but before we change we need to know this will not cause problems for others.

@jame
Copy link
Contributor Author

jame commented Aug 28, 2013

On Mon, 2013-08-26 at 09:50 -0700, Kevin Robert Keegan wrote:

Didn't this originally arise in the porting to debian?

That's where I originally ran across the issue, yes; multiple
instances of different scripts with different shabangs...

Is this allowed in debian packages?

Not in official ones; i.e., intended to be uploaded to Debian. By
policy[1], perl programs provided in Debian packages "must" use the
'/usr/bin/perl' path for the shabang line. In general I like to follow
Debian policy even for unofficial packages (which I also do) but in this
case I am interested in getting MH uploaded as an official package.

RJ Clay
[email protected]
[email protected]

[1]
http://www.debian.org/doc/packaging-manuals/perl-policy/ch-programs.html

@jame
Copy link
Contributor Author

jame commented Aug 28, 2013

On Mon, 2013-08-26 at 14:02 -0700, Lieven Hollevoet wrote:

After reading some more on this: apparently the opinions on hardcoding
either /usr/bin/perl or /usr/bin/env perl in scripts are very mixed.

Indeed...

The most sane/safe advice I found was that the installer should find
out what perl to use and should replace the shebang line with the
correct content when installing the script(s).

IIRC, that's configurable in some way, at least with the 'standard'
installers. (Not something I've had a chance to experiment with as yet,
though I plan to...)

However, since MisterHouse has no installer this is currently not
really on option to go for.

But perhaps something to plan for?

I think the first mention of this shebang line was indeed when @jame
was cleaning out the various shebangs we had for various scripts when
packaging for debian,

And for the standard Debian packaging I'm working on; if the path
gets standardized as something besides /usr/bin/perl and it's still not
configurable for the install, it's easy enough to have the packaging
patch the scripts that need the change.

but since the current shebang causes parts of MisterHouse to fail on
non-standard installs (i.e. systems that don't use the system-provided
Perl) I raised it again.

Non-standard installs are not something I've experimented much with
myself but I can see the point.

RJ Clay
[email protected]
[email protected]

@jame
Copy link
Contributor Author

jame commented Aug 28, 2013

On Mon, 2013-08-26 at 14:02 -0700, Lieven Hollevoet wrote:

I'm unsure how to proceed but I agree with @krkeegan we need to make a
decision on this issue.

Having a project wide policy I also think would be best, whatever it
turns out to be.

I'm in favor of /usr/bin/env perl as it allows the user to select
what perl to run

If that's the way it ends up going, then won't we need to ensure that
information about it is included in the documentation?

but before we change we need to know this will not cause problems for
others.

Not really a 'problem' for the Debian packaging, as it's easy enough
to patch the files if necessary. I understand 'env' is not always
in /usr/bin/, but I don't know that we can account for all possible
installs... And btw; how would be usable on MS Windows systems?

RJ Clay
[email protected]

@hollie
Copy link
Owner

hollie commented Aug 28, 2013

Hey RJ,

Op 28-aug.-2013, om 08:19 heeft Robert James Clay [email protected] het volgende geschreven:

On Mon, 2013-08-26 at 14:02 -0700, Lieven Hollevoet wrote:

I'm unsure how to proceed but I agree with @krkeegan we need to make a
decision on this issue.

Having a project wide policy I also think would be best, whatever it
turns out to be.

I agree.

I'm in favor of /usr/bin/env perl as it allows the user to select
what perl to run

If that's the way it ends up going, then won't we need to ensure that
information about it is included in the documentation?

Yes, sure. When we implement this we should document it.

but before we change we need to know this will not cause problems for
others.

Not really a 'problem' for the Debian packaging, as it's easy enough
to patch the files if necessary. I understand 'env' is not always
in /usr/bin/, but I don't know that we can account for all possible
installs... And btw; how would be usable on MS Windows systems?

Windows: would be very usable. There is no /usr/bin/perl on Windows either and it works too due to the binding of .pl files to the Perl interpreter.

Kind regards,
Lieven.

@hollie
Copy link
Owner

hollie commented Aug 28, 2013

Op 28-aug.-2013, om 08:19 heeft Robert James Clay [email protected] het volgende geschreven:

On Mon, 2013-08-26 at 14:02 -0700, Lieven Hollevoet wrote:

The most sane/safe advice I found was that the installer should find
out what perl to use and should replace the shebang line with the
correct content when installing the script(s).

IIRC, that's configurable in some way, at least with the 'standard'
installers. (Not something I've had a chance to experiment with as yet,
though I plan to...)

However, since MisterHouse has no installer this is currently not
really on option to go for.

But perhaps something to plan for?

Yes, if/when I find time I plan to experiment with Dist::Zilla to see how easy it is to integrate the existing codebase into a Dist::Zilla flow. This would bring us automatic dependency checking and lots of possibilities to configure what shebang gets implemented when a user installs the code package.

Kind regards,
Lieven.

@mstovenour
Copy link
Collaborator

Is this issue resolved? If not would it be possible to resolve it soon, before we make the next release?

@hollie
Copy link
Owner

hollie commented Dec 29, 2013

There is a proposed fix for the check email script in hollie/misterhouse
but it is not merged into master yet because there was no conclusive
feedback on the proposed fix.

Regards,
Lieven
Op 29 dec. 2013 16:49 schreef "Michael Stovenour" <[email protected]

:

Is this issue resolved? If not would it be possible to resolve it soon,
before we make the next release?


Reply to this email directly or view it on GitHubhttps://github.com//issues/141#issuecomment-31319224
.

@marcmerlin
Copy link
Collaborator

Quite frankly, I'm fine with anything consistent that works on Mac/Windows. On linux, it's trivial to make whatever solution work, either because it just does, or with a symlink.
I didn't quite see what fix you needed review on still, although if it's Mac/Win related, I'm not the best person to review, but if you're pretty sure it makes thing better for you and doesn't really break linux, I'd say go ahead and submit

@hollie
Copy link
Owner

hollie commented Dec 30, 2013

Hey Marc,

I will only have time to merge this after next weekend.

There is a branch called fix_get_email in the repo where I applied the fix
for a single script.

Could you test this out? If it works then I can apply it for the other
scripts too.

Regards,
Lieven
Op 30 dec. 2013 10:17 schreef "Marc MERLIN" [email protected]:

Quite frankly, I'm fine with anything consistent that works on
Mac/Windows. On linux, it's trivial to make whatever solution work, either
because it just does, or with a symlink.
I didn't quite see what fix you needed review on still, although if it's
Mac/Win related, I'm not the best person to review, but if you're pretty
sure it makes thing better for you and doesn't really break linux, I'd say
go ahead and submit


Reply to this email directly or view it on GitHubhttps://github.com//issues/141#issuecomment-31338232
.

@marcmerlin
Copy link
Collaborator

On Mon, Dec 30, 2013 at 03:50:48AM -0800, Lieven Hollevoet wrote:

Hey Marc,

I will only have time to merge this after next weekend.

There is a branch called fix_get_email in the repo where I applied the fix
for a single script.

Could you test this out? If it works then I can apply it for the other
scripts too.

I'm not at home for a while, so I'd rather not touch my mh tree and
upset the self running house for now, sorry :)
(not afraid about your change, but my tree is fairly behind)

Marc

"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
.... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/ | PGP 1024R/763BE901

@john-
Copy link

john- commented Dec 30, 2013

I am not configured to check email via mh. Is there a basic test I can do on linux to validate? I see the main bin/mh script has the hard coded path. I can change that to test if that helps.

@hollie
Copy link
Owner

hollie commented Jan 5, 2014

You can see the change I propose here:
1973c12

Basically replace the hard-coded "/usr/bin/perl” shebang with "/usr/bin/env perl"

This change allows the user to override the perl binary that is used without having to change symlinks in folders where a normal user does not have access.

Kind regards,
Lieven.

Op 31-dec.-2013, om 00:00 heeft john- [email protected] het volgende geschreven:

I am not configured to check email via mh. Is there a basic test I can do on linux to validate? I see the main bin/mh script has the hard coded path. I can change that to test if that helps.


Reply to this email directly or view it on GitHub.

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

6 participants