Hi Branden, G. Branden Robinson wrote on Thu, Mar 31, 2022 at 11:33:52PM +1100: > At 2022-03-30T13:25:24+0200, Ingo Schwarze wrote:
>> 6. DOCFILES_NOINST needs to be added to EXTRA_DIST unconditionally. > I believe Automake already has an idiom for this, the "noinst_" prefix. > https://www.gnu.org/software/automake/manual/html_node/Uniform.html I wasn't aware of "noinst_", so i looked at the page you pointed to, and found that it says: The special prefix ‘noinst_’ indicates that the objects in question should be built but not installed at all. None of the *.in files we are talking about need to be *built*, all four of them are already checked into the git repo. > Maybe it can't be applied to this situation; I'm not sure. Consequently, i think the "noinst_" prefix would be inadequate four these four files. For files that already exist in git and need to be added to the distribution tarball without being installed, EXTRA_DIST is the best option i'm aware of. Apart from that, we already use EXTRA_DIST in many places, whereas we only use noinst_LIBRARIES (which matches an example provided in the automake manual) and "nodist_noinst_HEADERS = defs.h", which is also a generated file. So far, we do not use it for files that are not built. >> Tested in builds both from git and from tarballs. >> >> When committing, i will include ChangeLog entries. >> >> OK to push? > Please look into the noinst thing; apart from that I'm a +1 on all > of this. Do my above findings imply that you are OK with my original patch? (appended again for convenience) Yours, Ingo P.S. I dislike how these *.am files are typically first defining variables with ad hoc, unsystematic names (for example SBOXES_NORMAL_FILES, SBOXES_PROCESSEDEXAMPLEFILES), then copying the contents of these into systematically named variables (in these two examples, into dist_sboxestmac_DATA and nodist_sboxesprocessedexample_DATA, respectively). That adds yet another, IMHO gratuitious layer of additional complexity with an additional risk of terminological confusion. It might be better to define systematically named variables right away, use them throughout, and delete the unsystematic names, except in those cases where a name is unsystematic on purpose in order to not trigger any automake magic (for example SBOXES_EXAMPLEFILES). But cleaning up the redundant naming should probably not be mixed into this bugfix patch. diff --git a/contrib/sboxes/sboxes.am b/contrib/sboxes/sboxes.am index 8eaf3a698..f3fb63dba 100644 --- a/contrib/sboxes/sboxes.am +++ b/contrib/sboxes/sboxes.am @@ -29,21 +29,16 @@ EXTRA_DIST += \ contrib/sboxes/ChangeLog \ contrib/sboxes/notquine.sed -# Files installed in $(examplesdir)/sboxes. SBOXES_EXAMPLEFILES are -# located in the source tree, while SBOXESPROCESSEDEXAMPLEFILES are -# generated in the build tree. SBOXES_EXAMPLEFILES = $(sboxes_srcdir)/msboxes.ms.in if BUILD_EXAMPLES sboxesexampledir = $(exampledir)/sboxes -dist_sboxesexample_DATA = $(SBOXES_EXAMPLEFILES) SBOXES_PROCESSEDEXAMPLEFILES = $(sboxes_builddir)/msboxes.ms \ $(sboxes_builddir)/msboxes.pdf sboxesprocessedexampledir = $(exampledir)/sboxes nodist_sboxesprocessedexample_DATA = $(SBOXES_PROCESSEDEXAMPLEFILES) -else +endif EXTRA_DIST += \ contrib/sboxes/msboxes.ms.in -endif MOSTLYCLEANFILES += msboxes.ms \ $(SBOXES_PROCESSEDEXAMPLEFILES) diff --git a/doc/doc.am b/doc/doc.am index 544fe147a..0c8c4daae 100644 --- a/doc/doc.am +++ b/doc/doc.am @@ -51,13 +51,14 @@ DOC_GROFF = nlinit=`echo 'nl="'; echo '"'`; eval "$$nlinit"; \ # Other doc, installed in $(docdir) if BUILD_OTHERDOC # Files located in the source tree -DOCFILES= \ +DOCFILES_INST= \ doc/me-revisions \ - doc/meintro.me.in \ - doc/meintro_fr.me.in \ - doc/meref.me.in \ doc/ms.ms \ doc/pic.ms +DOCFILES_NOINST= \ + doc/meintro.me.in \ + doc/meintro_fr.me.in \ + doc/meref.me.in # Files that undergo a transformation prior to groff processing GENERATEDDOCFILES = \ $(DOC_GNU_EPS) \ @@ -76,12 +77,13 @@ PROCESSEDDOCFILES = $(PROCESSEDDOCFILES_PS) \ doc/groff-man-pages.utf8.txt otherdocdir = $(docdir) -dist_otherdoc_DATA = $(DOCFILES) $(GENERATEDDOCFILES) +dist_otherdoc_DATA = $(DOCFILES_INST) $(GENERATEDDOCFILES) nodist_otherdoc_DATA = $(PROCESSEDDOCFILES) MOSTLYCLEANFILES += $(PROCESSEDDOCFILES) else -EXTRA_DIST += $(DOCFILES) $(GENERATEDDOCFILES) +EXTRA_DIST += $(DOCFILES_INST) $(GENERATEDDOCFILES) endif +EXTRA_DIST += $(DOCFILES_NOINST) # pdf doc, written in mom and therefore using contrib/mom/mom.am # definitions