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

fix blacklist in alienfile #1

Merged
merged 7 commits into from Jul 10, 2018
Merged

fix blacklist in alienfile #1

merged 7 commits into from Jul 10, 2018

Conversation

plicease
Copy link
Member

@plicease plicease commented Jul 9, 2018

When I copied this from the Makefile.PL in XML-LibXML I misunderstood how it worked. Though it makes sense now from the comments. Versions with a .1 suffix are good and .0 suffix are bad. Assume unmentioned are okay?

When I copied this from the Makefile.PL in XML-LibXML I misunderstood how it worked.  Though it makes sense now from
the comments.  Versions with a .1 suffix are good and .0 suffix are bad.  Assume unmentioned are okay?
@plicease plicease requested a review from genio July 9, 2018 01:36
@plicease
Copy link
Member Author

I've added logic to skip the system versions of libxml2 that are known to be bad unless $ENV{FORCE} is true (this is the env var that the Makefile.PL from XML-LibXML uses).

Unfortunately, I think the actual version list for the blacklist is wrong. Looking again at the Makefile.PL from XML-LibXML I noticed:

https://metacpan.org/source/SHLOMIF/XML-LibXML-2.0132/Makefile.PL#L167

So for example, the version of libxml2 in my OS X (High Sierra) is 2.9.4, so this alienfile does a share install unless $env{FORCE} is true, but XML-LibXML installs just fine. I think we just need to fix the list blacklisted versions, I think the logic is otherwise correct.

they set the version in the gather stage.  This should probably
be fixed in AB, at least optionally.  In the meanwhile, XML-LibXML
doesn't actually probe for libxml2 using pkg-config, so it isn't
atm any worse than XML-LibXML, it is just that it could be better.
Copy link
Collaborator

@genio genio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these changes look good to me. I plan on going through the rest of the Makefile.PL options tonight to see what else we may need to shunt in here.

@plicease plicease merged commit 137f6c8 into master Jul 10, 2018
@plicease plicease deleted the graham/fix-blacklist branch July 10, 2018 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants