Bug #111984 for Dist-Zilla-Plugin-Run: Need to run late, but before final packaging

Fri Feb 12 13:34:11 2016 tlhackque [...] yahoo.com - Ticket created

Subject:

Need to run late, but before final packaging

I need to run some fixup on certain files late in the build process. The (one) point I need is both before the .tar file is created and [CopyFilesFromBuild]. Can there be a [Run::BeforePackaging]? I'm not a Dist::Zilla expert, but I think this means making it a FileMunger. In my case, I need it to run after [Pod::Weaver] to inspect what it did & move some output to a non-pod file, which is packaged and [CopyFilesFromBuild] back to the top. But there are other uses - e.g. to sign a file and package the signature. [Run::somthing] would be a lot simpler than learning to write a custom plugin. Thanks.

Fri Feb 12 13:50:27 2016 ether [...] cpan.org - Correspondence added

There is a 'BeforeArchive' phase, which the [Run] plugin suite could certainly provide a hook into. I don't quite understand what you're trying to do though, so this might not be the best place in your situation. It's run well after everything else, including 'AfterBuild'. (AfterBuild is even too late to modify file contents before they are written to disk.) To modify files in the build, that would be a FileMunger, yes... if you're creating new files in the build, you'd need to insert something into the FileGatherer phase, which is even earlier than FileMunger. Did you know about https://metacpan.org/pod/Dist::Zilla::Plugin::Signature ?

Fri Feb 12 13:50:27 2016 The RT System itself - Status changed from 'new' to 'open'

Fri Feb 12 13:50:54 2016 ether [...] cpan.org - Severity Normal changed to Wishlist

Fri Feb 12 14:22:31 2016 tlhackque [...] yahoo.com - Correspondence added

On Fri Feb 12 13:50:27 2016, ETHER wrote: Show quoted text

> There is a 'BeforeArchive' phase, which the [Run] plugin suite could > certainly provide a hook into. I don't quite understand what you're > trying to do though, so this might not be the best place in your > situation. It's run well after everything else, including > 'AfterBuild'. (AfterBuild is even too late to modify file contents > before they are written to disk.) > > To modify files in the build, that would be a FileMunger, yes... if > you're creating new files in the build, you'd need to insert something > into the FileGatherer phase, which is even earlier than FileMunger. > > Did you know about > https://metacpan.org/pod/Dist::Zilla::Plugin::Signature ?

Thanks for the quick response! Obviously, I don't understand Dist::Zilla's internals. I'll try to clarify what I need to do. Today's use case is fairly simple. Pod::Weaver generates POD based on the build, e.g. a section for where bug reports go, the Version, copyright, Author. I want to place that into the README, which pod2readme should extract from the woven pod. Currently, it can't. So I have to maintain that separately. The README needs to end up in the archive. The README is also [CopyFromBuild] to the root so it is visible to GitHub. RunBeforeBuild is too soon to capture the woven pod. I thought AfterBuild runs too late - but I may be wrong. Yes, I even use Signature - that signs the distribution. So the README has to be created before the distribution signature file is created. There are some cases where one wants to sign a file created in the build. For example, where the file will be sent to a third party after the distribution is unpacked. Suppose I wanted to sign the created README with a differnt GPG key from that used by the distribution... (I don't, but more realistic examples are complicated...) I do know there's a plugin for README from POD. I'm not sure it can be scheduled after pod::weaver. But in any case, I have a local issue that requires the build to run a version of perl too old for that plugin. So I currently [Run:BeforeBuild] perlbrew exec -with the right version. And live with manually editing an include file before running the build. Life sure does get complicated.... :-(

Fri Feb 12 15:31:29 2016 ether [...] cpan.org - Correspondence added

On 2016-02-12 11:22:31, tlhackque wrote: Show quoted text

> Pod::Weaver generates POD based on the build, e.g. a section for where > bug reports go, the Version, copyright, Author. > > I want to place that into the README, which pod2readme should extract > from the woven pod. Currently, it can't. So I have to maintain that > separately.

Can you clarify why it cannot? Also, https://metacpan.org/pod/Dist::Zilla::Plugin::ReadmeAnyFromPod exists -- I use it in my plugin bundle like this to create the file in the repository only: [ReadmeAnyFromPod] :version = 0.142180 type = pod location = root phase = release Show quoted text

> The README needs to end up in the archive. > > The README is also [CopyFromBuild] to the root so it is visible to > GitHub.

Show quoted text

> RunBeforeBuild is too soon to capture the woven pod.

Right. Show quoted text

> I thought AfterBuild runs too late - but I may be wrong.

It is to get the file into the build -- since other plugins (e.g. [Manifest]) would want to know about that file's existence too. Show quoted text

> I do know there's a plugin for README from POD. I'm not sure it can > be scheduled after pod::weaver.

Most definitely it can -- I use podweaver heavily in my normal distribution packaging process. Show quoted text

> But in any case, I have a local issue > that requires the build to run a version of perl too old for that > plugin.

Doh! But I think ReadmeAnyFromPod works on 5.008 -- were you thinking of ReadmeFromPod?

Fri Feb 12 16:00:19 2016 tlhackque [...] yahoo.com - Correspondence added

On Fri Feb 12 15:31:29 2016, ETHER wrote: Show quoted text

> On 2016-02-12 11:22:31, tlhackque wrote: >

> > Pod::Weaver generates POD based on the build, e.g. a section for > > where > > bug reports go, the Version, copyright, Author. > > > > I want to place that into the README, which pod2readme should extract > > from the woven pod. Currently, it can't. So I have to maintain that > > separately.

> > Can you clarify why it cannot? >

Because I can't [Run] it at the right time :-) Show quoted text

> Also, https://metacpan.org/pod/Dist::Zilla::Plugin::ReadmeAnyFromPod > exists -- I use it in my plugin bundle like this to create the file in > the repository only: > > [ReadmeAnyFromPod] > :version = 0.142180 > type = pod > location = root > phase = release >

> > The README needs to end up in the archive. > > > > The README is also [CopyFromBuild] to the root so it is visible to > > GitHub.

> > RunBeforeBuild is too soon to capture the woven pod.

> > Right. >

> > I thought AfterBuild runs too late - but I may be wrong.

> > It is to get the file into the build -- since other plugins (e.g. > [Manifest]) would want to know about that file's existence too. >

> > I do know there's a plugin for README from POD. I'm not sure it can > > be scheduled after pod::weaver.

> > Most definitely it can -- I use podweaver heavily in my normal > distribution packaging process. >

> > But in any case, I have a local issue > > that requires the build to run a version of perl too old for that > > plugin.

> > Doh! But I think ReadmeAnyFromPod works on 5.008 -- were you thinking > of ReadmeFromPod?

ReadMeAnyFromPod uses Pod::Readme. Version 0.11 of PodReadme works with 5.008, but doesn't generate Markdown. V1.1.2 generates markdown, but requires Perl 5.10.something; I run just the pod2readme in a perlbrew 5.22.1 sandbox. The other challenge is that some content needs to go in the readme, but not the man pages. Pod::Weaver doesn't understand =begin readme blocks, except to skip them. So a fixup that handles the relocation is in order. Something like podweaver, pod2readme -f pod => runscript( move sections from woven pod to readme blocks ) => pod2text, pod2markdown. Somewhat painful, but less so than the "update copyrights" sessions. And so I'm here :-)

Fri Feb 12 16:29:28 2016 tlhackque [...] yahoo.com - Correspondence added

By the way, the MANIFEST shouldn't be a tough constraint. It's easy enough to touch README BeforeBuild, then fix-up later. That way the MANIFEST has a placeholder. The more restrictive timing problem is getting the right version into SIGNATURE. Of course, I hope that the hook placement is useful to others. Thanks for helping. There would seem to be a fairly steep learning curve for Dist::Zilla internals.

Fri Feb 12 16:53:37 2016 ether [...] cpan.org - Correspondence added

On 2016-02-12 13:29:28, tlhackque wrote: Show quoted text

> Because I can't [Run] it at the right time :-)

Ah yes. Show quoted text

> ReadMeAnyFromPod uses Pod::Readme.

Are you sure? I was able to install and test Dist::Zilla::Plugin::ReadmeAnyFromPod on a 5.8.8 system, and I don't see Pod::Readme in its (immediate) prereqs. I'm in the podlators group; if the 5.010 constraint is a trivial one I can see about getting that fixed up. Show quoted text

> The other challenge is that some content needs to go in the readme, but not the man pages. Pod::Weaver doesn't understand =begin readme blocks, except to skip them. So a fixup that handles the relocation is in order. Something like podweaver, pod2readme -f pod => runscript( move sections from woven pod to readme blocks ) => pod2text, pod2markdown. Somewhat painful, but less so than the "update copyrights" sessions.

This one sounds a bit more complicated... Show quoted text

> Thanks for helping. There would seem to be a fairly steep learning > curve for Dist::Zilla internals.

Truer words were never spoken :) BTW, if you ever use irc, there's usually someone around on irc.perl.org in the #distzilla channel. There may yet be a solution that I haven't yet fathomed :)

Fri Feb 12 18:17:37 2016 tlhackque [...] yahoo.com - Correspondence added

On Fri Feb 12 16:53:37 2016, ETHER wrote: Show quoted text

> On 2016-02-12 13:29:28, tlhackque wrote: >

> > Because I can't [Run] it at the right time :-)

> > Ah yes. >

> > ReadMeAnyFromPod uses Pod::Readme.

> > Are you sure? I was able to install and test > Dist::Zilla::Plugin::ReadmeAnyFromPod on a 5.8.8 system, and I don't > see Pod::Readme in its (immediate) prereqs. >

Too many similar names. Pod::Readme's pod2readme is what I've been using, including it's syntax for including other files (partially) and conditional inclusion within POD blocks. I have module source files with POD that conditionally generates different content for README and module doc. E.g. The module doc (e.g. perldoc foo::bar) doesn't need installation info (it's obviously already installed), but the readme does. The readme doesn't need method documentation, but the module doc does. The text README gets an "automagically generated by" line, but the Markdown for GitHub doesn't want that. But they share overview, description, recent changes and other significant sections. There's more in common than different. With everything in the module file, there's a much better chance of it staying current and in sync. And Pod::Weaver deals with stuff across files. Dist::Zilla::Plugin::ReadmeFromPod uses Pod::Readme. So the conditional POD should work - if it would run under DistZilla under Perl 5.8.8. I looked at Dist::Zilla::Plugin::ReadmeAnyFromPod *Any*. It seems to have no facility for selective inclusion. But it's doc page includes: "Dist::Zilla::Plugin::ReadmeFromPod - The base for this module" That's what made me think it relied on Pod::Readme. Apparently "base" doesn't mean in the OO class sense... more in the 'you look like my grandfather, could we be related' sense :-) Show quoted text

> I'm in the podlators group; if the 5.010 constraint is a trivial one I > can see about getting that fixed up. >

I didn't look at that, but I doubt it. From the Pod::Readme Changes, it appears to be a complete rewrite. The formerly simple module now relies on many other modules. Maybe it's just // vs || and some regexps. But there are a lot of modules go through. The code MOOved on. I installed into a fresh brew & the list of dependent modules and their builds was endless. (As in ~ an hour on a fast machine.) I think we made a mistake in the 60s when K lines of code became a 'productivity' metric. But that boat sailed, and I digress. v1.0.2 2014-10-13 22:18 BST - No changes since v1.0.1_08, just a version bump for a release. - See the full list of changes since v1.0.0 for more details. [Enhancements] - This is a complete rewrite, using modern Perl with Moo. - Added support for plugins, along with plugins to insert the module version, prerequisites and the latest changes. - Added the ability to generate a README in a variety of formats, such as POD, Markdown, HTML, RTF, etc. - Added command-line options to the pod2readme script, including the ability to specify the output format. - Switched to semantic versioning. - The documentation has been updated accordingly. [Incompatabilities] - Perl v5.10.1 is required. - This is no longer a subclass of a POD parser, although it has some wrapper methods for compatability with software known to use it. Show quoted text

> > The other challenge is that some content needs to go in the readme, > > but not the man pages. Pod::Weaver doesn't understand =begin readme > > blocks, except to skip them. So a fixup that handles the relocation > > is in order. Something like podweaver, pod2readme -f pod => > > runscript( move sections from woven pod to readme blocks ) => > > pod2text, pod2markdown. Somewhat painful, but less so than the > > "update copyrights" sessions.

> > This one sounds a bit more complicated...

Yup. But it's a trivial script. Something like: local $/; my $file = <>; my( $stuff ) = $file =~ /^(=head1 AUTHORS\n.*?)^=cut\n/ to pick up the block and$ $file =~ s/(*.*)^(=end readme\n)/$1$stuff$2/ms print $file; It's just getting to run the code in the right place... Have I explained enough of the constraints for you to identify the right place for a hook? Is this something you're willing to do? I'm open to other ideas, but so far this still seems like the least work for everyone... Show quoted text

> > Thanks for helping. There would seem to be a fairly steep learning > > curve for Dist::Zilla internals.

> > Truer words were never spoken :) > > BTW, if you ever use irc, there's usually someone around on > irc.perl.org in the #distzilla channel. There may yet be a solution > that I haven't yet fathomed :)

I rarely use IRC. I suspect that if I were willing to invest in learning Dist::Zilla internals, it could be done with a plugin. But that does seem like a lot of work - and a unique solution. An Run:: hook would work for many other use cases. I suppose I could ask the Dist::Zilla::Plugin::ReadmeAnyFromPod maintainer to add conditional inclusion. It would be a good thing. But more work.

Fri Feb 12 18:57:49 2016 ether [...] cpan.org - Correspondence added

On 2016-02-12 15:17:37, tlhackque wrote: Show quoted text

> > I'm in the podlators group; if the 5.010 constraint is a trivial one > > I > > can see about getting that fixed up. > >

> > I didn't look at that, but I doubt it. > > From the Pod::Readme Changes, it appears to be a complete rewrite. > The formerly simple module now relies on many other modules. Maybe > it's just // vs || and some regexps.

I took a quick look at the Pod-Readme repository and the only 5.10isms I could find was a lot of 'state' variables -- but that can be patched with local lexicals like so: -sub Indentation { - state $type = Type::Tiny->new( +{ + my $type; + sub Indentation { + return $type if $type; + $type = Type::Tiny->new( ... - return $type; + } } Show quoted text

> Have I explained enough of the constraints for you to identify the > right place for a hook? Is this something you're willing to do? > I'm open to other ideas, but so far this still seems like the least > work for everyone...

I can add a BeforeArchive plugin to [Run], but it would be pretty similar to running during AfterBuild, and even that's probably too late. But it's still a good idea to have one, so I'll see about doing that tonight/this weekend. Show quoted text

> I suppose I could ask the Dist::Zilla::Plugin::ReadmeAnyFromPod > maintainer to add conditional inclusion. It would be a good thing. > But more work.

He's good people; he'll listen (I've arm-wrestled him into releasing patches in the past) :)

Sat Feb 13 11:54:33 2016 tlhackque [...] yahoo.com - Correspondence added

On Fri Feb 12 18:57:49 2016, ETHER wrote: Show quoted text

> On 2016-02-12 15:17:37, tlhackque wrote: >

> > > I'm in the podlators group; if the 5.010 constraint is a trivial > > > one > > > I > > > can see about getting that fixed up. > > >

> > > > I didn't look at that, but I doubt it. > > > > From the Pod::Readme Changes, it appears to be a complete rewrite. > > The formerly simple module now relies on many other modules. Maybe > > it's just // vs || and some regexps.

> > I took a quick look at the Pod-Readme repository and the only 5.10isms > I could find was a lot of 'state' variables -- but that can be patched > with local lexicals like so: > > -sub Indentation { > - state $type = Type::Tiny->new( > +{ > + my $type; > + sub Indentation { > + return $type if $type; > + $type = Type::Tiny->new( > ... > - return $type; > + } > }

Ah, state. Pod::Readme now uses MOO and a bunch of other modules, so you might be opening a larger project that first appears... Show quoted text

> > Have I explained enough of the constraints for you to identify the > > right place for a hook? Is this something you're willing to do? > > I'm open to other ideas, but so far this still seems like the least > > work for everyone...

> > I can add a BeforeArchive plugin to [Run], but it would be pretty > similar to > running during AfterBuild, and even that's probably too late. > > But it's still a good idea to have one, so I'll see about doing that > tonight/this weekend.

Thanks. That would be good. The point I need is after PodWeaver and before Signature. Careful placement in dist.ini is a reasonable constraint. Show quoted text

> > I suppose I could ask the Dist::Zilla::Plugin::ReadmeAnyFromPod > > maintainer to add conditional inclusion. It would be a good thing. > > But more work.

> > He's good people; he'll listen (I've arm-wrestled him into releasing > patches in the past) :)

https://rt.cpan.org/Ticket/Display.html?id=112004 We'll see how he responds... Thanks for all your time and support!

Sun Feb 14 09:07:06 2016 tlhackque [...] yahoo.com - Correspondence added

Subject:

Naive thought

On Fri Feb 12 18:57:49 2016, ETHER wrote: Show quoted text

> I can add a BeforeArchive plugin to [Run], but it would be pretty > similar to > running during AfterBuild, and even that's probably too late. >

I may not understand the complexities, but: As Dist::Zilla::Plugin::ReadmeAnyFromPod can be scheduled at the 'right' time by dist.ini placement, can't a Run::During (Or Run::Later, or?) be scheduled the same way? You are clearly the expert, but I'm a little curious...

Mon Feb 15 16:59:33 2016 ether [...] cpan.org - Correspondence added

On 2016-02-14 06:07:06, tlhackque wrote: Show quoted text

> On Fri Feb 12 18:57:49 2016, ETHER wrote:

> > I can add a BeforeArchive plugin to [Run], but it would be pretty > > similar to > > running during AfterBuild, and even that's probably too late. > >

> I may not understand the complexities, but: > > As Dist::Zilla::Plugin::ReadmeAnyFromPod can be scheduled at the > 'right' time by dist.ini placement, can't a Run::During (Or > Run::Later, or?) be scheduled the > same way? > > You are clearly the expert, but I'm a little curious...

Yes, quite possibly, if you run at the right phase! I've created [Run::BeforeArchive] -- let me know how it goes.

Mon Feb 15 16:59:35 2016 ether [...] cpan.org - Status changed from 'open' to 'resolved'

Mon Feb 15 16:59:35 2016 ether [...] cpan.org - Fixed in 0.043 added

Wed Feb 17 12:29:09 2016 tlhackque [...] yahoo.com - Correspondence added

Subject:

[Run::BeforeArchive} results: working, but ugly w/ some issues

On Mon Feb 15 16:59:33 2016, ETHER wrote: Show quoted text

> On 2016-02-14 06:07:06, tlhackque wrote:

> > On Fri Feb 12 18:57:49 2016, ETHER wrote:

> > > I can add a BeforeArchive plugin to [Run], but it would be pretty > > > similar to > > > running during AfterBuild, and even that's probably too late. > > >

> > I may not understand the complexities, but: > > > > As Dist::Zilla::Plugin::ReadmeAnyFromPod can be scheduled at the > > 'right' time by dist.ini placement, can't a Run::During (Or > > Run::Later, or?) be scheduled the > > same way? > > > > You are clearly the expert, but I'm a little curious...

> > Yes, quite possibly, if you run at the right phase! > > I've created [Run::BeforeArchive] -- let me know how it goes.

Many thanks for the new plugin. You said: "quite possibly, if you run at the right phase!". But I don't think that the Run plugin lets me control the phase in which it executes my script :-) I have things working, but it's not pretty. Here are the results of my exploration: Setup: [Run::BeforeBuild] ; MANIFEST needs to see output files. ; Delete existing to make sure what's left after build came from this build. ; This complicates things later because pod2readme uses mtime to avoid building. run = rm -f README README.md; touch README README.md ... [PodWeaver] [Run::BeforeArchive] run = sed -i -e'/^=head1 AUTHOR/i=for readme start\n\n' %d%plib%pMy%pModule.pm ; -F because of touch run = ./runbrew -in %d pod2readme -F lib%pMy%pModule.pm && cp -p %d%pREADME . run = ./runbrew -in %d pod2readme -F -f markdown lib%pMy%pModule.pm && cp -p %d%pREADME.md . ... [MetaYAML] [MetaJSON] [Manifest] [CopyFilesFromBuild] [Signature] ... Good news: 1) BeforeArchive runs after [PodWeaver] 2) BeforeArchive runs before [Signature] Non-ideal news: 1) BeforeArchive runs after [Manifest], which causes a mismatch complaint and the files don't get archived. Touching README and README.md in [Run::BeforeBuild] works around this. But, pod2readme notes the READMEs are newer than the source file & silently refuses to build them. [and 'optimization') So, -F is required. It would be simpler if one could do something to register files created by the script for the manifest. Perhaps: [Run::BeforeArchive] adds_file = README adds_file = README.md This could be a no-op if the named file already exists. 2) BeforeArchive runs after [CopyFilesFromBuild] Arguably, this is an issue with [CopyFilesFromBuild] - running too soon... I work-around that by copying my output as part of the command. 3) %d is documented as 'not in "BeforeBuild'. Using %d in before build should produce an error. The current behavior is to ignore it, which means a command can access an unintended path. E.g. %d%pfoo will access /foo. 4) If I don't remove the build directory before building, there is no 'mismatch' complaint. This seems to indicate that dzil isn't emptying the build directory at the start of a build. This seems like a bad thing, as it means that repeated builds can accumulate stale files. Notes: a) Documentation nit: %v isn't available in [Run::BeforeBuild] as documented, because it comes from [VersionFromModule]. And [VersionFromModule] looks in the [empty] build directory. Which isn't there yet. Bottom line: I have something that works, but there's room for improvement. Thanks again for making the progress possible.

Wed Feb 17 12:29:11 2016 tlhackque [...] yahoo.com - Status changed from 'resolved' to 'open'

Wed Feb 17 12:29:11 2016 tlhackque [...] yahoo.com - Given to ETHER

Wed Feb 17 17:04:51 2016 ether [...] cpan.org - Correspondence added

On 2016-02-17 09:29:09, tlhackque wrote: Show quoted text

> Touching README and README.md in [Run::BeforeBuild] works around this. > But, pod2readme notes the READMEs are newer than the source file & > silently refuses to build them. [and 'optimization') So, -F is > required. > > It would be simpler if one could do something to register files > created > by the script for the manifest. > Perhaps: > [Run::BeforeArchive] > adds_file = README > adds_file = README.md > This could be a no-op if the named file already exists.

That won't make any difference at all in the result. The file needs to be added in the FileGatherer phase (which is right after [Run::BeforeBuild] runs), no matter who does it. Show quoted text

> 2) BeforeArchive runs after [CopyFilesFromBuild] Arguably, this is an > issue with [CopyFilesFromBuild] - running too soon...

Yes, [CopyFilesFromBuild] runs in the AfterBuild phase, which is before BeforeArchive. I would suggest using [CopyFilesFromRelease] instead, which will be late enough. You can also use [Git::Commit] at that time to commit the file to your repository so you don't need to add a blank one again next time around. But, if you run your command in the *build* directory, rather than in the repository, the file will exist and you won't have to depend on [CopyFilesFrom*] to bring the file to you. Show quoted text

> I work-around that by copying my output as part of the command. > > 3) %d is documented as 'not in "BeforeBuild'. Using %d in before > build > should produce an error. The current behavior is to ignore it, > which > means a command can access an unintended path. E.g. %d%pfoo will > access /foo.

What do you mean by ignore it? %d should evaluate to the empty string in BeforeBuild. Show quoted text

> 4) If I don't remove the build directory before building, there is no > 'mismatch' > complaint. This seems to indicate that dzil isn't emptying the > build > directory at the start of a build. This seems like a bad thing, as > it means that repeated builds can accumulate stale files.

Yes, that sounds bad, and a worthy issue to file for Dist-Zilla. (Which I've done here -- https://github.com/rjbs/Dist-Zilla/issues/521) Show quoted text

> a) Documentation nit: > %v isn't available in [Run::BeforeBuild] as documented, because > it comes from [VersionFromModule]. And [VersionFromModule] looks > in the [empty] build directory. Which isn't there yet.

%v asks for $zilla->version; it's the plugin's responsibility to provide that as soon as it's able. There's nothing that [Run::BeforeBuild] can do about that, but it can be noted in the documentation.

Wed Feb 17 18:33:07 2016 tlhackque [...] yahoo.com - Correspondence added

On Wed Feb 17 17:04:51 2016, ETHER wrote: Show quoted text

> On 2016-02-17 09:29:09, tlhackque wrote: >

> > Touching README and README.md in [Run::BeforeBuild] works around > > this. > > But, pod2readme notes the READMEs are newer than the source file & > > silently refuses to build them. [and 'optimization') So, -F is > > required. > > > > It would be simpler if one could do something to register files > > created > > by the script for the manifest. > > Perhaps: > > [Run::BeforeArchive] > > adds_file = README > > adds_file = README.md > > This could be a no-op if the named file already exists.

> > That won't make any difference at all in the result. The file needs to > be > added in the FileGatherer phase (which is right after > [Run::BeforeBuild] > runs), no matter who does it.

Yes, but can't Run::BeforeArchive do that as a FileGatherer? There doesn't seem to be an external way to say "This file is going to exist, so look for it when you MANIFEST/sign/archive". And it seems reasonable that one reason to run a script is to create a file... So specifying what it produces in the same place where it's told to run seems reasonable. Show quoted text

> > 2) BeforeArchive runs after [CopyFilesFromBuild] Arguably, this is > > an > > issue with [CopyFilesFromBuild] - running too soon...

> > Yes, [CopyFilesFromBuild] runs in the AfterBuild phase, which is > before > BeforeArchive. > > I would suggest using [CopyFilesFromRelease] instead, which will be > late > enough. You can also use [Git::Commit] at that time to commit the > file to > your repository so you don't need to add a blank one again next time > around.

I think that CopyFilesFromRelease only runs if one does a release. I like to check the README before exposing it. So I'll do multiple trial builds, then a test. Finally a TRIAL release to cpan. And only then a release. So CopyFilesFromRelease is too late in the process. Sigh. It's not obvious what the harm would be in CopyFilesFromBuild running later, unless the file it wants to copy is removed by cleanup between where it runs now and 'later'. I still make mistakes, but I do try to minimize how much they're exposed. The README in the repo is always the last one released, so one can see what was packaged. And also so when you open the project on GitHub, you see it as the "home page". Show quoted text

> > But, if you run your command in the *build* directory, rather than in > the > repository, the file will exist and you won't have to depend on > [CopyFilesFrom*] to bring the file to you.

Actually, it's the other way around. I do run the command in the build directory. that's the -in %d to runbrew. The dummy blank file is to satisfy Manifest. I delete the old one so if something goes wrong, there's a zero length file, not some confusion from an old one. The reason to copy it to the top level is because that's where the git repo is, so the latest README gets pushed to the remote repo - GitHub in this case - when I commit. The build directories and tar files are all in .gitignore If I were to build it in the top level, it wouldn't go in the kit, and I'd have all the MANIFEST issues in spades. Plus, the Pod::Weaver output is only in the distribution directory. Show quoted text

> > I work-around that by copying my output as part of the command. > > > > 3) %d is documented as 'not in "BeforeBuild'. Using %d in before > > build > > should produce an error. The current behavior is to ignore it, > > which > > means a command can access an unintended path. E.g. %d%pfoo will > > access /foo.

> > What do you mean by ignore it? %d should evaluate to the empty string > in > BeforeBuild.

It does. And that's the problem. If you inadvertently use %d in BeforeBuild, it evaluates to the empty string instead of throwing an error. So, if you thought %d%pfoo would go to (or read from) your distribution directory, well, it goes to the root directory (/) instead. Clearly you can't do what was requested, so rather than a silent failure, an error seems in order. Show quoted text

> > 4) If I don't remove the build directory before building, there is no > > 'mismatch' > > complaint. This seems to indicate that dzil isn't emptying the > > build > > directory at the start of a build. This seems like a bad thing, > > as > > it means that repeated builds can accumulate stale files.

> > Yes, that sounds bad, and a worthy issue to file for Dist-Zilla. > (Which I've done here -- https://github.com/rjbs/Dist- > Zilla/issues/521)

Thanks. Show quoted text

> > a) Documentation nit: > > %v isn't available in [Run::BeforeBuild] as documented, because > > it comes from [VersionFromModule]. And [VersionFromModule] > > looks > > in the [empty] build directory. Which isn't there yet.

> > %v asks for $zilla->version; it's the plugin's responsibility to > provide that > as soon as it's able. There's nothing that [Run::BeforeBuild] can do > about > that, but it can be noted in the documentation.

Agree.

Wed Feb 17 19:33:00 2016 ether [...] cpan.org - Correspondence added

On 2016-02-17 15:33:07, tlhackque wrote: Show quoted text

> Yes, but can't Run::BeforeArchive do that as a FileGatherer?

[Run::BeforeArchive] runs at the BeforeArchive phase, not the FileGatherer phase. Show quoted text

> There > doesn't seem to be an external way to say "This file is going to > exist, so look for it when you MANIFEST/sign/archive".

That's what FileGatherers are for. Show quoted text

> > I would suggest using [CopyFilesFromRelease] instead, which will be > > late > > enough. You can also use [Git::Commit] at that time to commit the > > file to > > your repository so you don't need to add a blank one again next time > > around.

> > I think that CopyFilesFromRelease only runs if one does a release. I > like to check the README before exposing it. So I'll do multiple > trial builds, then a test. Finally a TRIAL release to cpan. And only > then a release. > So CopyFilesFromRelease is too late in the process. Sigh.

Too late for what? You don't need a plugin to copy a file for you so you can inspect it. Just inspect it in the build directory!! $ dzil build $ less Foo-Bar-1.23/README Show quoted text

> It's not obvious what the harm would be in CopyFilesFromBuild running > later, unless the file it wants to copy is removed by cleanup between > where it runs now and 'later'.

CopyFilesFromBuild runs at the AfterBuild phase. That's plenty late enough. Show quoted text

> The README in the repo is always the last one released, so one can see > what was packaged. And also so when you open the project on GitHub, > you see it as the "home page".

Obviously. Show quoted text

> > > > But, if you run your command in the *build* directory, rather than in > > the > > repository, the file will exist and you won't have to depend on > > [CopyFilesFrom*] to bring the file to you.

> > Actually, it's the other way around.

No. Re-read what I said. Show quoted text

> The reason to copy it to the top level is because that's where the git > repo is, so the latest README gets pushed to the remote repo - GitHub > in this case - when I commit.

I fully understand that -- which is why I suggested using [Git::Commit] to commit it to the repository for you so you don't have to manually run 'git commit' after releasing. Show quoted text

> If I were to build it in the top level, it wouldn't go in the kit,

I never suggested doing that. Show quoted text

> It does. And that's the problem. If you inadvertently use %d in > BeforeBuild, it > evaluates to the empty string instead of throwing an error. So, if > you thought > %d%pfoo would go to (or read from) your distribution directory, well, > it goes to the root directory (/) instead.

Okay, a warning would be reasonable there, rather than silently providing ''. I'm closing this ticket now as [Run::BeforeArchive] has been provided, and I don't think I can help you further.

Wed Feb 17 19:33:02 2016 ether [...] cpan.org - Status changed from 'open' to 'resolved'