PerlAlien/Alien-Build

Generated Install/Files.pm can't be overridden

djerius opened this issue · 8 comments

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]

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.pm changes.

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?

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.

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.

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.

@djerius can you try it with 3e3d53a and let me know if that does what you need?

@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.

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

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