Bug #122661 for SVG-Timeline: scripts should use /usr/bin/env perl

Mon Jul 31 09:01:58 2017 kevin [...] 27escape.com - Ticket created

Subject:	scripts should use /usr/bin/env perl
Date:	Mon, 31 Jul 2017 14:01:14 +0100
To:	bug-SVG-Timeline [...] rt.cpan.org
From:	Kevin Mulholland <kevin [...] 27escape.com>

Hi Dave, I know its a small thing but could you have your timeline script use #!/usr/bin/env perl rather than #!/usr/bin/perl This allows me to use perlbrew rather than relying on system perl Many thanks Kevin (cpan: MOODFARM, twitter @27escape)

Mon Jul 31 11:27:39 2017 DAVECROSS [...] cpan.org - Taken

Mon Jul 31 11:28:21 2017 DAVECROSS [...] cpan.org - Correspondence added

Version 0.0.3 is en route to CPAN now and contains this fix. Thanks for the suggestion. Dave...

Mon Jul 31 11:28:21 2017 The RT System itself - Status changed from 'new' to 'open'

Mon Jul 31 13:36:16 2017 rt-cpan [...] trout.me.uk - Correspondence added

On Mon Jul 31 11:28:21 2017, DAVECROSS wrote: Show quoted text

> Version 0.0.3 is en route to CPAN now and contains this fix. > > Thanks for the suggestion.

The distribution will work fine with perlbrew without this "fix". What you actually did was break it for people using perlbrew properly, since it may end up running against a perl that doesn't have the module or its dependencies installed. Please revert this change, it's completely wrong.

Tue Aug 01 04:31:21 2017 kevin [...] 27escape.com - Correspondence added

Subject:	Re: [rt.cpan.org #122661] scripts should use /usr/bin/env perl
Date:	Tue, 1 Aug 2017 09:30:47 +0100
To:	bug-SVG-Timeline [...] rt.cpan.org
From:	Kevin Mulholland <kevin [...] 27escape.com>

Hi Dave, Sorry for the misguided suggestion and causing you extra work, it seems that Matt is right (as ever). I did not know that the installer re-writes /usr/bin/perl to be local perl in use, in my case to #!/home/kmulholland/perl5/perlbrew/perls/perl-5.23.4/bin/perl5.23.4 /usr/bin/env perl should only then be used for standalone scripts that do not go through the install process. regards kevin On 31 July 2017 at 18:36, Matt S Trout via RT <bug-SVG-Timeline@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=122661 > > > On Mon Jul 31 11:28:21 2017, DAVECROSS wrote:

> > Version 0.0.3 is en route to CPAN now and contains this fix. > > > > Thanks for the suggestion.

> > The distribution will work fine with perlbrew without this "fix". > > What you actually did was break it for people using perlbrew properly, > since it may end up running against a perl that doesn't have the module or > its dependencies installed. > > Please revert this change, it's completely wrong. >

Tue Aug 01 04:51:18 2017 DAVECROSS [...] cpan.org - Correspondence added

To be honest, I'm not convinced it is a misguided suggestion. I mean, yes, it sounds like it's not needed for perlbrew users, but I'm not just supporting perlbrew users. I've been in the process of ensuring that all programs in my CPAN distributions use "env" rather than a hard-coded path and your ticket reminded me that I'd got it wrong in this distribution. My gut reaction is that the "env" approach is still the way to go. But I need to think about it more. Dave... On Tue Aug 01 04:31:21 2017, kevin@27escape.com wrote: Show quoted text

> Hi Dave, > > Sorry for the misguided suggestion and causing you extra work, it seems > that Matt is right (as ever). I did not know that the installer re-writes > /usr/bin/perl to be local perl in use, in my case to > #!/home/kmulholland/perl5/perlbrew/perls/perl-5.23.4/bin/perl5.23.4 > > /usr/bin/env perl should only then be used for standalone scripts that do > not go through the install process. > > regards > kevin > > On 31 July 2017 at 18:36, Matt S Trout via RT <bug-SVG-Timeline@rt.cpan.org> > wrote: >

> > <URL: https://rt.cpan.org/Ticket/Display.html?id=122661 > > > > > On Mon Jul 31 11:28:21 2017, DAVECROSS wrote:

> > > Version 0.0.3 is en route to CPAN now and contains this fix. > > > > > > Thanks for the suggestion.

> > > > The distribution will work fine with perlbrew without this "fix". > > > > What you actually did was break it for people using perlbrew properly, > > since it may end up running against a perl that doesn't have the module or > > its dependencies installed. > > > > Please revert this change, it's completely wrong. > >

Tue Aug 01 11:58:50 2017 rt-cpan [...] trout.me.uk - Correspondence added

On Tue Aug 01 04:51:18 2017, DAVECROSS wrote: Show quoted text

> > To be honest, I'm not convinced it is a misguided suggestion. I mean, > yes, it sounds like it's not needed for perlbrew users, but I'm not > just supporting perlbrew users. > > I've been in the process of ensuring that all programs in my CPAN > distributions use "env" rather than a hard-coded path and your ticket > reminded me that I'd got it wrong in this distribution.

That's ... really bad. When you have a 'normal' hard-coded path in your distributions then EUMM/MB/etc. replace it with the path to the perl you're installing to, which means that you're guaranteed the script will get the versions of the modules it was installed against. using 'env' for applications you're deploying from git is often very nice, but if you're *installing* something it's much better to make sure it runs against the correct perl.

Tue Aug 01 12:05:01 2017 rt-cpan [...] trout.me.uk - Correspondence added

On Tue Aug 01 04:31:21 2017, kevin@27escape.com wrote: Show quoted text

> /usr/bin/env perl should only then be used for standalone scripts that do > not go through the install process.

This is indeed the recommended rule and what the toolchainers have spent several years filing bugs against various distributions to remove 'env' from installable scripts for. It would be unfortunate for us to now go backwards from that point. Also, 'env perl' can be loltacularly bad in unexpected ways for users with more than one perl - for example, if I install a script to one perl and then change $PATH or perlbrew switch or whatever to a different perl, that script is now going to fail because it doesn't have the underlying module. My copy of e.g. ~/bin/ack should not be broken by 'perlbrew switch' - and isn't, because ack lets the toolchain do its job. Were ack to use 'env perl' then I would have to make sure I had the exact same version of App::Ack installed to all of my perls or risk my 'ack' script breaking. This would not be a feature. Dave, hopefully the above makes it clear why this is a terrible idea. I'm open to further discussion if not. (btw, I *used* to use env in distributable modules myself and switched because I managed to get myself into the equivalent of the above situation and ended up with holes in my feet - so please consider avoiding learning it the hard way like I did :)

Tue Aug 01 12:27:13 2017 DAVECROSS [...] cpan.org - Correspondence added

On Tue Aug 01 12:05:01 2017, MSTROUT wrote: Show quoted text

> On Tue Aug 01 04:31:21 2017, kevin@27escape.com wrote:

> > /usr/bin/env perl should only then be used for standalone scripts > > that do > > not go through the install process.

> > This is indeed the recommended rule and what the toolchainers have > spent several years filing bugs against various distributions to > remove 'env' from installable scripts for. > > It would be unfortunate for us to now go backwards from that point. > > Also, 'env perl' can be loltacularly bad in unexpected ways for users > with more than one perl - for example, if I install a script to one > perl and then change $PATH or perlbrew switch or whatever to a > different perl, that script is now going to fail because it doesn't > have the underlying module. My copy of e.g. ~/bin/ack should not be > broken by 'perlbrew switch' - and isn't, because ack lets the > toolchain do its job. Were ack to use 'env perl' then I would have to > make sure I had the exact same version of App::Ack installed to all of > my perls or risk my 'ack' script breaking. This would not be a > feature. > > Dave, hopefully the above makes it clear why this is a terrible idea. > I'm open to further discussion if not. (btw, I *used* to use env in > distributable modules myself and switched because I managed to get > myself into the equivalent of the above situation and ended up with > holes in my feet - so please consider avoiding learning it the hard > way like I did :)

Yeah. I see all that and I'm convinced. I'll get a new version out as soon as I can. Is there a toolchain recommendations document somewhere, where someone who doesn't go to the QA Hackathons and apparently doesn't have time to keep up with all the right mailing lists can go for a snapshot of recommended best practices on this stuff?

Tue Aug 01 15:00:18 2017 kevin [...] 27escape.com - Correspondence added

Subject:	Re: [rt.cpan.org #122661] scripts should use /usr/bin/env perl
Date:	Tue, 1 Aug 2017 19:59:03 +0100
To:	bug-SVG-Timeline [...] rt.cpan.org
From:	Kevin Mulholland <kevin [...] 27escape.com>

The only discussions I could find were from 2013, not that that is a bad thing, but I could not find anything like a perl article or blog saying this is the right way to do things, esp from the plenv/perlbrew folks. We discussed it at work and everyone had the 'use env' mindset, we have all now changed our minds! I need to update my few CPAN dists soon anyway and will also change them to be correct Thanks for SVG::Timeline btw, its not something I can use currently as it is not granular enough but I keep my eyes peeled whenever a new SVG module pops up. And thanks Matt for taking the time to educate us! cheers kevin On 1 August 2017 at 17:27, Dave Cross via RT <bug-SVG-Timeline@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=122661 > > > On Tue Aug 01 12:05:01 2017, MSTROUT wrote:

> > On Tue Aug 01 04:31:21 2017, kevin@27escape.com wrote:

> > > /usr/bin/env perl should only then be used for standalone scripts > > > that do > > > not go through the install process.

> > > > This is indeed the recommended rule and what the toolchainers have > > spent several years filing bugs against various distributions to > > remove 'env' from installable scripts for. > > > > It would be unfortunate for us to now go backwards from that point. > > > > Also, 'env perl' can be loltacularly bad in unexpected ways for users > > with more than one perl - for example, if I install a script to one > > perl and then change $PATH or perlbrew switch or whatever to a > > different perl, that script is now going to fail because it doesn't > > have the underlying module. My copy of e.g. ~/bin/ack should not be > > broken by 'perlbrew switch' - and isn't, because ack lets the > > toolchain do its job. Were ack to use 'env perl' then I would have to > > make sure I had the exact same version of App::Ack installed to all of > > my perls or risk my 'ack' script breaking. This would not be a > > feature. > > > > Dave, hopefully the above makes it clear why this is a terrible idea. > > I'm open to further discussion if not. (btw, I *used* to use env in > > distributable modules myself and switched because I managed to get > > myself into the equivalent of the above situation and ended up with > > holes in my feet - so please consider avoiding learning it the hard > > way like I did :)

> > Yeah. I see all that and I'm convinced. I'll get a new version out as soon > as I can. > > Is there a toolchain recommendations document somewhere, where someone who > doesn't go to the QA Hackathons and apparently doesn't have time to keep up > with all the right mailing lists can go for a snapshot of recommended best > practices on this stuff? > > >

Sat Aug 19 12:16:59 2017 DAVECROSS [...] cpan.org - Status changed from 'open' to 'resolved'