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

rewrite #!/usr/bin/env perl #58

Open
miyagawa opened this issue Apr 29, 2013 · 47 comments
Open

rewrite #!/usr/bin/env perl #58

miyagawa opened this issue Apr 29, 2013 · 47 comments
Labels

Comments

@miyagawa
Copy link
Member

(Somewhat related to #37 and #38?)

Is there any reason that ExtUtils::MakeMaker fixin() rewrites #!/usr/bin/perl or #!perl but NOT #!/usr/bin/env perl?

  1. There are many reasons #!/usr/bin/env perl do not work when installed. Keeping that line in the shebang sounds wrong to me.

  2. Yet, having #!/usr/bin/env perl is useful for scripts that you want to run from a git repository etc. Look what I do with cpanm bin scripts so that you can run from a git repo by cp, but also installable to the right perl path, by rewriting the shebang in Makefile.PL: https://github.com/miyagawa/cpanminus/blob/devel/Makefile.PL#L9

There might be a valid use case for allowing #!/usr/bin/env perl as stated in #38 - so it might make sense to "allow" it by leaving as is if @INC is configured as relocatable, but by default, it makes more sense to rewrite it upon install, the same way it does with #!perl.

@karenetheridge
Copy link
Member

For scripts that you want to run from a git repository, what's wrong with doing perl bin/plackup? (or if running from a unit test, $^X bin/plackup?)

I have some repositories where I split up my "scripts for aiding the development process" and "scripts I want installed into $PATH" into separate directories, and EUMM only points to one of these in EXE_FILES. Sometimes I write a wrapper script in the former directory (which might use just #!perl or #!/usr/bin/env perl) that call scripts in the latter, via exec ^$X <path>.

@miyagawa
Copy link
Member Author

For scripts that you want to run from a git repository, what's wrong with doing perl bin/plackup?

Nothing's wrong with that, and that's orthogonal to this issue.

The thing is, I can have #!perl or #!/usr/bin/perl, the former doesn't work for anybody (unless you have ./perl in the current directory), and the latter works for many people unless your primary perl isn't /usr/bin/perl (i.e. perlbrew etc.). Either way it will be fixed upon installation, so I have to tell people to not run them as a command, even though they have +x bit.

I wish I could write #!/usr/bin/env perl in there so that it will work for even more people, and then rewritten upon installation. Puzzled why it's not supported.

@kraih
Copy link

kraih commented Apr 29, 2013

👍 There are hundreds (maybe thousands) of scripts already using #!/usr/bin/env perl on CPAN. http://grep.cpan.me/?q=%2Fusr%2Fbin%2Fenv+perl

@karenetheridge
Copy link
Member

Personally I don't find the "but everyone is doing it" argument very compelling.

@miyagawa
Copy link
Member Author

Personally I don't find the "but everyone is doing it" argument very compelling.

It's not "everyone's doing it so we should do it" - it's "everyone's doing it hoping it works (without knowing it isn't supported)", and that's why the default should change to support it.

@jberger
Copy link
Member

jberger commented Apr 30, 2013

👍 for rewriting! I assumed (apparently incorrectly) that using env was a good idea for cpan-installable scripts. :-/ I can imagine many others falling into that trap!

https://github.com/jberger/Galileo/blob/master/bin/galileo#L1

@Leont
Copy link
Member

Leont commented Apr 30, 2013

I've always used #!perl because I am not expecting the script to work out of the box (nor do I set an executable bit). For that matter, «#!/usr/bin/env perl» wouldn't work on non-unix operating systems anyway.

@miyagawa
Copy link
Member Author

I've always used #!perl because I am not expecting the script to work out
of the box (nor do I set an executable bit). For that matter,
«#!/usr/bin/env perl» wouldn't work on non-unix operating systems anyway.

#!/usr/bin/perl won't either but that gets rewritten. And env perl doesn't.


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

@kraih
Copy link

kraih commented Apr 30, 2013

@Leont Are you arguing for the removal of #!/usr/bin/perl rewriting and making #!perl the one true choice?

@timbunce
Copy link

timbunce commented May 1, 2013

+1 for @miyagawa's proposal.

@Leont
Copy link
Member

Leont commented May 1, 2013

Are you arguing for the removal of #!/usr/bin/perl rewriting and making #!perl the one true choice?

No, I'm saying the general expectation is silly and wrong.

That doesn't mean that this can't be a useful feature, but I think it should be framed in a more general discussion of "what should we do if the shebang line is not targeting something that looks like a perl binary"

@schwern
Copy link
Contributor

schwern commented May 5, 2013

Let me try and untangle what's going on here.

First, so everyone's on the same page, MakeMaker has a feature where it will rewrite #!/path/to/something/like/perl to be #!/path/to/the/perl/you/ran/the/Makefile/PL/with. This happens in ExtUtils::MM_Unix->fixin. This is necessary because it assumes (correctly in just about every case) that the program needs the modules being installed and those modules will only be reliably available to the perl which ran the Makefile.PL. Even if the script doesn't need modules, it's still a reliable way to find a path to a working perl. This has been happening for years and years and it only affects programs which actually get installed.

@miyagawa has noticed that fixin does not see #!/usr/bin/env perl as a perl binary and so does not rewrite it. This is most likely a simple oversight in the fixin code which has been pretty much unchanged since the Pink Camel book. He's proposing we change fixin to recognize #!/usr/bin/env perl and rewrite it to be #!/path/to/the/perl/you/ran/the/Makefile/PL/with just like any other perl program.

The rub is this: Unlike all the other hard coded versions, which will not work reliably on other people's machines, #!/usr/bin/env perl is the way you say "find perl in your PATH". Was the intent of the person writing #!/usr/bin/env perl to really have their script work with the perl in the user's PATH? Or did they assume MakeMaker would fix the #! line like it does everything else? Or, and this is the most likely option, do they have no idea MakeMaker is fixing their #! line because it just works and never had to think about it?

As @kraith pointed out, this is a big problem.

That's the issue as I see it. LMK if that summary missed anything. Proposed solution next.

@shadowcat-mst
Copy link
Member

I do this regularly for pure perl scripts that I want bundled into
a relocatable local::lib.

I'd be happy to see an option to MakeMaker to produce the behaviour that
miyagawa wants.

Changing the current behaviour would be a regression from my POV.

@schwern
Copy link
Contributor

schwern commented May 5, 2013

Lemme lay out my assumptions. If the assumptions aren't correct, my whole proposal doesn't work.

Assumption 1, most folks who write #! lines aren't even thinking about portability. MakeMaker has taken care of that for so long the feature is invisible. And that's fine, one less thing for an author to worry about and screw up.

Assumption 2, there are some people who write #!/usr/bin/env perl expecting that MakeMaker will not alter the #! line because they want it to run on the first perl in PATH.

Assumption 2a, but most of them aren't.

Assumption 3, we can't guess which is which.

When you have an overwhelming majority of users doing X and a small majority wanting Y which is in conflict with X, the usual solution is to make the default X and make it configurable. Right now, fixin just happens whether you want it to or not.

Here's what I propose.

  1. Add a switch to turn fixin on or off (defaulting to on).
  2. Change fixin so it will see #!/usr/bin/env perl as perl.
  3. Allow the user to pass in a subroutine which rewrites the #! line.

The first is pretty straight forward and lets people who want all the scripts to keep their #! lines do that in an easy fashion. SHBANG_FIX => 0 in the WriteMakefile. (Not "FIXIN" because if you don't already know that name you're unlikely to guess it)

The second lets most authors continue to not care about any of this and write their #! lines in the new fashion.

The third accommodates people who have more complicated needs. I see it working like this...

SHBANG_REPLACE => sub {
    my %shbang = @_;

    ...alter %shbang...

    return \%shbang;
};

%shbang would contain things like the file and line number, the full line, just the path part, the perl interpreter and any switches. We want to handle all the tokenizing for the user and hand them tokens. The user can then change entries in %shbang and return it.

I think that accommodates everyone's needs. If that's cool, I'll break this up into three issues for work. This one has gotten a bit long.

@schwern
Copy link
Contributor

schwern commented May 5, 2013

@shadowcat-mst said

I do this regularly for pure perl scripts that I want bundled into a relocatable local::lib.

How a script is going to be used is a choice made not by the distribution author but by the user installing the distribution.

If you can write perl Makefile.PL SHBANG_FIX=0 would that solve your problem? We could even add in some way to specify perl Makefile.PL SHBANG_REPLACE_WITH='#!/usr/bin/env perl' as a special case of SHBANG_REPLACE so every script will have its #! rewritten to be relocatable. In fact, perl Makefile.PL RELOCATABLE=1 would probably be the best choice to encapsulate the feature in case we figure a better way to do it later and/or want to go beyond just the #! line.

How's that?

@shadowcat-mst
Copy link
Member

I can see adding that to PERL_MM_OPT and being happy, yeah.

Could we please, please call it SHEBANG_FIX though? That's the usual
spelling according to everything I've seen - and wikipedia, for what
that's worth - and I just -know- I'll typo it otherwise.

@schwern
Copy link
Contributor

schwern commented May 5, 2013

I bow to Wikipedia's authority. SHEBANG it is.

@miyagawa
Copy link
Member Author

miyagawa commented May 5, 2013

Here's what I propose.

Add a switch to turn fixin on or off (defaulting to on).
Change fixin so it will see #!/usr/bin/env perl as perl.
Allow the user to pass in a subroutine which rewrites the #! line.

Agreed 100% on the assumptions and +1 for the proposals (SHEBANG_FIX=0 etc.). Sounds great!

@Leont
Copy link
Member

Leont commented May 7, 2013

The third accommodates people who have more complicated needs. I see it working like this...

Given that that subs needs to be given in the Makefile.PL, and fixin is run through make, that may get messy…

@Leont
Copy link
Member

Leont commented May 7, 2013

We should probably do something similar in Module::Build/Module::Build::Tiny. The former currently always rewrites, the latter does something similar to MakeMaker.

@ikegami
Copy link

ikegami commented Jun 4, 2013

I would imagine "#!/usr/bin/env perl" is used in one of the following three circumstances:

  1. The script isn't expected to be installed.
  2. The script is expected to be installed, but the author doesn't know about MM's rewriting. It just seemed like the most portable default to the author.
  3. Cargo-culting.

In all three cases, rewriting by default seems the correct approach.
+1, for whatever it's worth.

hirose31 added a commit to hirose31/mha4mysql-manager that referenced this issue Mar 31, 2014
ExtUtils::MakeMaker rewirtes #/usr/bin/perl or #!perl but NOT
#!/usr/bin/env perl, so mandatorily rewrite shebang if environment variable
REWRITE_SHEBANG is set.

- Perl-Toolchain-Gang/ExtUtils-MakeMaker#58
hirose31 added a commit to hirose31/mha4mysql-node that referenced this issue Mar 31, 2014
ExtUtils::MakeMaker rewirtes #/usr/bin/perl or #!perl but NOT
#!/usr/bin/env perl, so mandatorily rewrite shebang if environment variable
REWRITE_SHEBANG is set.

- Perl-Toolchain-Gang/ExtUtils-MakeMaker#58
@Leont Leont added the Wishlist label Feb 1, 2015
@mohawk2
Copy link
Member

mohawk2 commented May 10, 2018

That last one is what App::Prove currently does, right? Maybe that needs extracting into a separately-usable module, maybe something like lib::inherit::callperl(@argv)?

sjn pushed a commit to sjn/ExtUtils-MakeMaker that referenced this issue Jun 28, 2018
- Relocatable shebangs are nice. Let's try to make them happen!
- This PR is related to tickets Perl-Toolchain-Gang#38 and Perl-Toolchain-Gang#58
bingos pushed a commit that referenced this issue Jul 19, 2018
- Relocatable shebangs are nice. Let's try to make them happen!
- This PR is related to tickets #38 and #58
@sdondley
Copy link

So this is merged now? Just so I'm clear, this means a distribution developer can leave #!/usr/bin/env perl in a script and it will be changed to an appropriate path when installed by the end user without the developer having to do anything?

@Grinnz
Copy link
Contributor

Grinnz commented Oct 16, 2018

No, nothing has been done yet. Even if/when it's added to EUMM, Module::Build and other installers have to be fixed, and developers still can't use #!/usr/bin/env perl because the user probably won't have an up to date installer.

@sdondley
Copy link

Oh, OK, I mistook the purple Merged symbols for this project but they are for others.

@toddr
Copy link
Contributor

toddr commented Nov 28, 2018

I've read this thread twice just now and I'm still not sure I understand where everyone's at so let me see if I can summarize and get some people to yell at me and tell me I'm wrong :D

  1. The decision on what the shebang is should be in the power of the person installing the script, not the CPAN author. In most if not all cases, the CPAN author doesn't actually know what every person doing the install wants.
  2. People with a re-locatable perl need a way for the shebang to not be #!$^X.

It seems like the discussed solution would be to:

  1. Add #!/usr/bin/env perl to the patterns that leads to a #! rewrite.
  2. Continue to default the re-write to $^X
  3. Provide a means to alter the rewrite value to something else. Most people want /usr/bin/env perl but others might want something else. SHEBANG_FIX=1 produces /usr/bin/env perl or SHEBANG_FIX=/my/custom/path/to/whatever provides a way for them to be explicit about what they want it to be.

Right now everyone's unhappy. Some CPAN distros rewrite to $^X which angers the relocatable perl folks. Some CPAN distros use /usr/bin/env perl which frustrates the distros that don't want to have to be dependent on $PATH to get the perl script they explicitly just called to behave correctly.

Is anyone actually against this proposed fix?

@Grinnz
Copy link
Contributor

Grinnz commented Nov 28, 2018

SHEBANG_FIX refers to the behavior that it already currently does, so the name should be different, but otherwise 👍

@ap
Copy link

ap commented Nov 29, 2018

It seems like the discussed solution would be to:

That solution is mostly what was suggested at the top of the thread; most of the following conversation is not reflected. It does reflect some of the discussion in that it includes a way to change the rewritten value to something other than $^X. But missing from it is any mechanism to minimise disruption resulting from erasing the hitherto distinction between #!perl and #!/usr/bin/env perl.

Is anyone actually against this proposed fix?

I am. I am actually in favour of each one of the points in the proposal, but I am against the proposal as a whole, because it is incomplete. I imagine everyone else on the thread so far who voiced concerns about breakage feels the same way.

@toddr
Copy link
Contributor

toddr commented Nov 29, 2018

I am. I am actually in favor of each one of the points in the proposal, but I am against the proposal as a whole, because it is incomplete. I imagine everyone else on the thread so far who voiced concerns about breakage feels the same way.

Many of the comments above were answered and this ticketing system doesn't really allow for threading so it's not clear what's "unanswered". Could you please be clear what additional change / improvement would be needed?

@toddr
Copy link
Contributor

toddr commented Nov 29, 2018

But missing from it is any mechanism to minimise disruption resulting from erasing the hitherto distinction between #!perl and #!/usr/bin/env perl.

Sorry, I missed this part of your reply.

It's my opinion that the decision on what the shebang is should be in the power of the person installing the script, not the CPAN author. In most if not all cases, the CPAN author doesn't actually know what every person doing the install wants.

So my question to you: Under what circumstances would the user or distro maintainer actually benefit from some of the shebangs in a distro being $^X and some being /usr/bin/env perl? I can see a use for both but not in the same perl install.

If you come up with one, is there any reason the distro maintainer couldn't just patch that in their edge case? We're already planning to provide a command line option to specify the shebang in every module install.

@toddr
Copy link
Contributor

toddr commented Mar 4, 2022

What about support for an environment variable which tells the fixins the installer wants /usr/bin/env perl re-written to the perl used with Makefile.PL? As a distro maintainer, this would allow me to back out many of the patches I have to maintain related to this.

@Grinnz
Copy link
Contributor

Grinnz commented Mar 4, 2022

The vast majority of installers will not know they want this - shebang fix needs to "just work", but certainly there should be a way to skip rewriting, though my uninformed opinion is there should be an option to rewrite all shebangs to /usr/bin/env perl since many distros don't use this to begin with as rewriting is currently broken for it.

@toddr
Copy link
Contributor

toddr commented Mar 4, 2022

The big "But what about" seems to be relocatable perl. That's a %Config value. Maybe relocatable perl could preserve /usr/bin/env when that is the given perl?

@toddr
Copy link
Contributor

toddr commented Mar 4, 2022

The vast majority of installers will not know they want this.

I agree but this ticket doesn't seem to be going anywhere so I'm looking for a compromise. Perhaps cpan.pm could set the environment var, etc and we could move forward.

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

No branches or pull requests