search

PerlAlien / Alien-Build

Build external dependencies for use in CPAN
16 stars 25 forks source link

Generated Install/Files.pm can't be overridden #393

Closed djerius closed 1 year ago

djerius commented 1 year ago

I run perltidy tests as part of my release process, and the Install/files.pm file generated by Alien::Build::MM::build fails my particular brand of tidyness.

I'm more than happy to provide a replacement, but there doesn't seem to be a mechanism to do so.

[This is all done via Dist::Zilla::Plugin::Test::Perl::Critic, and their doesn't appear to be a mechanism to instruct it to ignore certain files]

djerius commented 1 year ago

A simple approach might be to check if the file is in the distribution and not do anything if it is.

This might cause problems in the future if the functionality of Install/Files.pmchanges.

Perhaps requiring a $ALIEN_BUILD_API_VERSION_FOR_INSTALL_FILES (or something suitably bike-shedded) variable be defined in Install/Files.pm and croaking if it return something that's older than what Alien::Build expects?

plicease commented 1 year ago

I'm sorry, I don't quite understand your suggestion.

My feeling is that [Test::Perl::Critic] should not be checking files in blib, though that will be hard to change now since all_critic_ok simply recurses from .. Failing that I think [Test::Perl::Critic] should provide an option to test specific files + directories instead of .. For an AB based alien you'd want lib and alienfile, for dists that have a script directory you would also want to test that as well.

djerius commented 1 year ago

I'm sorry, I don't quite understand your suggestion.

Sorry about that.

Install/Files.pm is generated unconditionally. Instead, only generate it if the author hasn't provided one.

plicease commented 1 year ago

Install/Files.pm is generated unconditionally. Instead, only generate it if the author hasn't provided one.

Got it, I can do that it and it seems reasonable.

Caveat: if we need to add features to ::Install::Files then stuff may break in the future, and IMO [Test::Perl::Critic] should be fixed. In practice I don't think we have changed the behavior of ::Install::Files since it was first implemented.

plicease commented 1 year ago

@djerius can you try it with https://github.com/PerlAlien/Alien-Build/pull/397/commits/3e3d53ad42cecc7122a024f2f47197b88f025775 and let me know if that does what you need?

plicease commented 1 year ago

@djerius I'm working on a release so I went ahead and merged #397, and will close this, but please LMK if this doesn't do what you need.

djerius commented 1 year ago

Hi, sorry for the long silence. This works for my needs. Thanks!

plicease commented 1 year ago

@djerius great! no worries. Thanks for the confirmation.