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

Patch dbicadmin support the #! Makefile.PL was invoked with #130

Closed
wants to merge 1 commit into from

Conversation

toddr
Copy link

@toddr toddr commented Nov 27, 2018

No description provided.

@ribasushi
Copy link
Collaborator

@toddr this patch can't be accepted. It assumes that every user wants this rewrite to take place unconditionally, removing access to the current local::lib-friendly behavior from users relying on it.

IIRC to get what you want the user needs to set the proper EU::MM environment variable ( I don't recall what was settled on ) at install time, so that the install process does the proper rewriting.

I will leave this PR open until we clarify whether the above works for you and/or whether I missed something.

@toddr
Copy link
Author

toddr commented Nov 28, 2018

I've patched around it so if you have good reasons for it that's fine but I'll re-iterate the argument I made for doing this in another case:

I'll give you a real work example. Let's say you install SQL::Translator to your perl in /usr/local. The assumption is that if I run /usr/local/bin/sqlt-diff, it'll work. However if my $PATH is set to pick perl out of /usr/bin first, then the script will fail because SQL::Translator was not installed to that perl.

So you want ExtUtils::MakeMaker to update the #! of all of the scripts during make install to point to the path of the perl they were installed to. It will not do this unless your #! is /usr/bin/perl in cpan distro.

https://metacpan.org/pod/ExtUtils::MakeMaker#EXE_FILES

@toddr
Copy link
Author

toddr commented Nov 28, 2018

@ribasushi I'm unaware of the local::lib problem. Can you point me to something that explains that issue please? Thanks!

@ribasushi
Copy link
Collaborator

@toddr Perl-Toolchain-Gang/ExtUtils-MakeMaker#58 (comment)

Also please read that same discussion starting from her: Perl-Toolchain-Gang/ExtUtils-MakeMaker#58 (comment)

This ( EU::MM ) would be the correct place to apply this fix, with a way for users being able to disable ( if default ) or enable ( if not ) this rewriting.

If this is really a showstopper, I would be amenable to accept a patch to Makefile.PL in the spirit of Perl-Toolchain-Gang/ExtUtils-MakeMaker#58 (comment) ( unless the EU::MM default behavior changes, this patch will have to be strictly opt-in with an envvar, preferably matching the one proposed for EU::MM )

@toddr
Copy link
Author

toddr commented Nov 28, 2018

Thanks I’ll patch my distro around it

@toddr toddr closed this Nov 28, 2018
@Grinnz
Copy link

Grinnz commented Nov 28, 2018

Note the referenced comment is not referring to standard usage of local::lib but specifically a local::lib to be relocatable into a perl installed somewhere else. The majority of CPAN scripts work properly when installed to a perl that is not at the beginning of PATH, and the majority of use cases are not relocatable perls, so until EUMM's behavior is improved in the suggested manner I don't feel it's worth keeping script installations broken for everyone else.

@ribasushi
Copy link
Collaborator

ribasushi commented Nov 28, 2018

A major selling point of CPAN is ( or was 😿 ) that it does not flail from one standard usage to the next based on the current majority of use cases.

If you strongly feel the current behavior is deathly wrong, please institute a defaults change at the core of EU::MM.

If this is not feasible - I provided a path forward with a non-default envvar regardless of EU::MM version.

Bottom line: the current default will not change unilaterally within this project.

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