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.

Sign up for GitHub

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

Jump to bottom

Extraneous "use Alien::Gnuplot 4.4;" requirements? #98

Closed
pjwelsh opened this issue Aug 16, 2023 · 13 comments · Fixed by #99
Closed

Extraneous "use Alien::Gnuplot 4.4;" requirements? #98

pjwelsh opened this issue Aug 16, 2023 · 13 comments · Fixed by #99

Comments

@pjwelsh
Copy link

pjwelsh commented Aug 16, 2023

blib/lib/PDL/Graphics/Gnuplot.pm
lib/PDL/Graphics/Gnuplot.pm
Both contain "use Alien::Gnuplot 4.4;" and should likely only be "use Alien::Gnuplot;".
This is causing cpanspec to determine a requirement that can not be met as the current Alien::Gnuplot version is just 1.042.
@drzowie
Copy link
Collaborator

drzowie commented Aug 16, 2023 via email

@pjwelsh
Copy link
Author

pjwelsh commented Aug 16, 2023

I may have slightly misspoken. The rpmbuild seems to be the one picking up on a false dependency from the two mentioned files. If I remove the "4.4", the package builds without issue.

@zmughal
Copy link
Member

zmughal commented Aug 16, 2023

Perhaps the change should just be to add a BEGIN { } block in this module that calls Alien::Gnuplot->VERSION() with the minimum gnuplot (the binary) version and change the Alien docs appropriately.

Though I believe that rpmbuild is being too clever here... all the version information for required modules comes from (MY)META.json files via Makefile.PL and CPAN itself uses the our $VERSION line when indexing versions. Scanning the source is more of a thing for developer tools, not packaging tools.

@pjwelsh
Copy link
Author

pjwelsh commented Aug 17, 2023

I'm not a Perl guy or dev guy and I just want to make sure we are on the same page...
When I look the perl "use" clause, it accepts the "version" as a version requirement for the imported function. The current max version for "Alien::Gnuplot" is "1.042". Both mentioned .pm files seem to be requiring a version of Alien::Gnuplot of "4.4" that does not exist.
rpmbuild does indeed poke into files to create requirement directives. Not perfect, but that is what it does to safeguard RPM package installs.

@drzowie
Copy link
Collaborator

drzowie commented Aug 17, 2023 via email

@pjwelsh
Copy link
Author

pjwelsh commented Aug 17, 2023 via email

@zmughal
Copy link
Member

zmughal commented Aug 17, 2023

Yes, we're doing something a bit non-standard here. Most modules do not override the default VERSION method in UNIVERSAL.

I see where rpmbuild does that module version extraction here. @jplesnik, have other modules done this and is there an easy way to prefer the requirements in the META.json? It may be uncommon enough that we can just go ahead and just change how we use it here and release a new version of PDL-Graphics-Gnuplot.

zmughal added a commit that referenced this issue Aug 17, 2023
@zmughal
Split version checking from use Alien::Gnuplot
0220b8e 
in order to make downstream packaging easier.

This works around dependency scanning code that assumes the standard
semantics where a version-like string in that location refers to the
version of the Perl module that is stored in the package variable
`$VERSION`.

In particular, this is what `rpmbuild` does via the `perl-generators`
plugin per <https://github.com/jplesnik/generators/blob/1.16/template/bin/perl.req#L240>.

Fixes <#98>.
@zmughal zmughal mentioned this issue Aug 17, 2023
Merged
@pjwelsh
Copy link
Author

pjwelsh commented Aug 17, 2023

I needed this module as part of dependency chain (from hell) of over 60 perl modules in order to build/install as requirement for AI::TensorFlow::Libtensorflow. I just thought reporting this may help someone else stuck in the same boat building this into a deployable RPM. Making the change is up to you, of course, but it would be good IMHO.
Thank you for working through this with me! Much appreciated.

@zmughal
Copy link
Member

zmughal commented Aug 17, 2023

AI::TensorFlow::Libtensorflow

That's my fault. ;-)

Thank you for reporting this. Anything that helps people install in more places and get work done is appreciated.

@pjwelsh
Copy link
Author

pjwelsh commented Aug 17, 2023

LOL did not even look at the author on that AI module... too funny.

@jplesnik
Copy link

@zmughal Using use Alien::Gnuplot 4.4; means that you need Alien::Gnuplot version 4.4 or later.

If you want to check version of gnuplot you should use
$Alien::Gnuplot::version - gets the self-reported version number of the executable.
See https://metacpan.org/pod/Alien::Gnuplot#DESCRIPTION

@pjwelsh
Copy link
Author

pjwelsh commented Aug 21, 2023

Not sure if it 100% the same, but there seems to be a similar usage for PDL-Graphics-Gnuplot-2.024/Makefile.PL:
"use ExtUtils::MakeMaker 6.48;"
Where 6.48 does not seem to be a valid version for ExtUtils::MakeMaker at https://metacpan.org/pod/ExtUtils::MakeMaker

@pjwelsh
Copy link
Author

pjwelsh commented Aug 21, 2023

Maybe never mind on the previous comment since since the "x version or later" still works it seems...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Split version checking from use Alien::Gnuplot PDLPorters/PDL-Graphics-Gnuplot
4 participants
@zmughal @drzowie @jplesnik @pjwelsh