Re: [PATCH] lib/readline/doc makefiles clean targets

[Mike Jonkmans]

On Mon, Nov 11, 2024 at 03:30:53PM -0500, Chet Ramey wrote:
> On 11/8/24 4:15 PM, Mike Jonkmans wrote:

> > > > 2) The use of recursive make, makes it harder to do dependencies right;
> > > It's the best way to build using optional components and subdirectories.
> > I have my doubts here. There are potential problems with dependencies
> > and parallel makes wrt linking.
> Well, sure. There are always potential problems. But we have to have a
> real build problem on a real system that's due to parallel make and
> dependencies, so we know whether or not we've fixed anything.

If things work for you, that's fine as it is the most important aspect.

> > > > 3) The various Makefiles disagree on 'clean: mostlyclean' versus
> > > >      'mostlyclean: clean';
> > > Is this substantive or just cosmetic?
> I can swap those around as I modify the individual makefiles; it's not a
> big deal.

Ok, nice.

> > > > 4) .PHONY is not used for the sub-makes *clean rules;
> > > You mean in the subdirectory makefiles?
> > One can argue about the substantive versus cosmetic aspect of .PHONY.
> > As it is highly unlikely to accidently create a file named after a target.
> > But they don't hurt - and FWIW may save a stat or two.
> Sure, I can add those as well.

Nice2

> > > > 5) git (status), .gitignore and make are disconnected;
> > > What does this mean? There are a couple of files that I need to add
> > > to .gitignore; is this something else?
> > I suggest per directory .gitignore files, that contain:
> This isn't really a problem description.

Automatically generating .gitignore from the Makefile would prevent
forgetting to add items.
It is not important, it is a suggestion.

> > > > 6) There is redundancy/duplication in the actions of various rules;
> > > OK.
> > > > 6) Same for the definitions of variables.
> > > Examples?
> > There is duplication in the top Makefile on the various lib sources.
> > One might dedup that by creating e.g. lib/glob/Makefile.ext and include
> > that on the toplevel (the lib's Makefile should then use GLOB_SOURCES).
> I don't really consider that a problem.

It is not an actual problem. But how sure are you that all dependencies
in the top Makefile are listed correctly? It is an enormous list.

> > lib/glob/Makefile:
> >     Seems that gmisc.c is missing from CSOURCES
> >     CSOURCES might then be derived from OBJECTS.
> The source files don't change often enough to make that work worth it.

On itself, it might not be worth the trouble.
Only in combination with deduplication.

> > The top Makefile defines GLOB_SOURCES, different from those CSOURCES.
> > Oh, i see, #include "glob_loop.c" etc. :(
> This is a fairly common idiom for building single-byte and multibyte
> versions of functions from the same source without duplicating too much
> code. glibc and gnulib both use it.

Weirdness all around ;)
These are header files. No actual problem, though.

> > Those *_loop.c files are not included in CSOURCES nor in HSOURCES.
> OK.
> > Could be a problem for the what-tar target (via THINGS_TO_TAR).
> Those targets aren't used; I inherited them from the original glob
> library. Bash distributions are built a different way.

How about removing unused targets and variables?
If THINGS_TO_TAR is removed, some more removals could follow.

> > > > > > (One could argue, whether the devel branch is a distribution.)
> > > > > It's not. It's a snapshot of ongoing work.
> > > > As such, it has the purpose of:
> > > > - bringing in early fixes/patches;
> > > > - communicating future changes, eliciting comments.
> > > In many cases, yes. In some cases, I solicit discussion, usually here.
> > > > So the devel branch should be easy to work with for outsiders.
> > > How would the standard make targets make it easier to work with (besides
> > > fixing a couple of previously-discussed issues)? What are we doing here
> > > beyond `configure; make' and testing out various behaviors?
> > Making people not chase red herrings. I mean that in the output of
> > 'make mostlylean', seeing 'clean' called, makes me want
> > to look at what is wrong.
> You're assuming something is wrong based on a set of assumptions about
> how things `should be', not whether anything is actually misbehaving.

Inconsistencies, such as mostlyclean/clean, may be a source for
future misbehaving. So they warrant some investigation.
Fixing it, prevents others looking at it again.

> > (From 'info (make)Standard Targets')
> > All GNU programs should have the following targets in their Makefiles:
> >     all install install-html install-dvi install-pdf install-ps
> >     install-strip uninstall
> >     clean distclean mostlyclean maintainer-clean
> >     TAGS info dvi html pdf ps dist check installcheck installdirs
> > The wording has 'should have'. But I think that if you have any of these
> > targets, they should be doing the expected.
> What is `the expected'? The only one we're really missing here is
> `installcheck', and few programs seem to have or use it (bash doesn't
> install anything but the man pages and info files).

There is not a 'must have' for these targets.

The info page describes what these targets ought to do.
That is what I would expect.
One may add that a subdir-make of such a standard target does "the expected"
in the subdir.

> > > > - 'make mostlyclean' removes doc/bashref.* and doc/Makefile.
> > > It shouldn't remove the Makefile, and you can make it again, but I can't
> > > reproduce it removing the bashref.* files.
> > On devel:
> >     make distclean
> >     git restore .
> >     ./configure
> >     make mostlyclean
> >     git status
> > Shows several lines of 'deleted: doc/bashref.*'
> Those are the files resulting from texinfo creating the dvi files from
> the texinfo source. They are not part of the bash distribution. We talked
> about them earlier.

Yes, but you say that you can not reproduce.

Thanks for changing some of the things I mentioned.

-- 
Regards, Mike Jonkmans