Re: [PATCH] newlib: merge iconvdata into top-level Makefile

Mike Frysinger <[email protected]> Tue, 25 Jan 2022 16:19:56 -0500
Newsgroups	gmane.comp.lib.newlib
Message-ID	<YfBpfKkcSK2i+/AD@vapier>
On 25 Jan 2022 13:27, Corinna Vinschen wrote:
> On Jan 24 19:57, Mike Frysinger wrote:
> > On 24 Jan 2022 17:45, Corinna Vinschen wrote:
> > > Looks good, except, there's this local.mk again.  I don't think this
> > > name is overly helpful.  All Makefiles, even those just included by the
> > > master Makefile should be called Makefile.somethingorother, IMHO.  These
> > > are much easier to find for people new to the stuff.
> > > 
> > > Same goes for the already existing newlib/doc/local.mk which I missed
> > > when reviewing the patches...
> > 
> > tl;dr: i've wrestled with this exact issue for a while now, before even
> > newlib, and imo "local.mk" is currently the least-worst option.  i did not
> > use that here lightly, or invent it myself.
> > 
> > there isn't strong style guidance as to which name to use.  it's already in
> > use in binutils & gdb, and other GNU projects like automake and autoconf.
> > 
> > the GNU standards document is silent on the matter unfortunately:
> > https://www.gnu.org/prep/standards/html_node/Makefile-Conventions.html
> > 
> > some other OSS projects have used the name "Makemodule.am", and i think i've
> > seen "module.am" once or twice.  i dislike these as i don't think using the
> > ".am" suffix is a good idea.  that implies automake itself will be processing
> > it which it doesn't.  the fact that it's included by Makefile.am somewhere is
> > incidental.  we have strong precedence that foo.in will produce foo, and that
> > foo.am will produce foo.in.  which doesn't happen here.
> > 
> > in general, i think we have pretty strong precedence that makefile frags
> > should be named ".mk".  that's a convention that goes back quite a long
> > way (i.e. BSD days), and is used commonly in GNU projects, including in
> > the toolchain repo now -- see /config/*.mk as many examples.
> > 
> > i'm acutely aware that we have multilib.am" in the root of the toolchain
> > projects, but that is the multilib logic that's kind of been bolted on in
> > places, so i wouldn't look to it for precedence.  imo it should get renamed
> > to "multilib.mk", but i don't have the willpower to shave that yak :p.
> > 
> > libgloss has a config/ subdir of makefile fragments that uses ".mh" and
> > ".mt" (for makefile host & target frags), but that convention isn't used
> > anywhere else, and seems like it's on the way out.
> > 
> > so if we've settled on ".mk" as the suffix, let's focus on the basename.
> > 
> > i don't think "Makefile.mk" is a good idea name as it implies a tight
> > coupling to a "Makefile" in the samedir.  i've also never really seen
> > that before.  along those lines, i'm not keen on "Make<anything>.mk".
> > 
> > i have seen "module.mk" once or twice.  that might be slightly better
> > than "local.mk".
> > 
> > so all things considered, i think "local.mk" is what we should use.
> 
> Local.mk is neither a name speaking for itself, nor does it stand out
> due to being one of very few files in a dir starting with uppercase, or
> due to its catchphrase "Make...".  In terms of easy recognition for
> people trying to wrap their head around the build system and trying to
> find the right place to add or improve build rules, local.mk isn't
> overly helpful.
> 
> The *.mk files in Free/Open/NetBSD are not just called local.mk and are
> spread out over all the subdirs.  Most of these files are in the sys/conf
> and /share/mk dirs and have a speaking name like kern.opts.mk, etc.
> 
> In the core kernel and lib subdirs all BSDs use files called Makefile.inc,
> actually.
> 
> Therefore I would prefer Makefile.inc, too, as name for the Makefile
> snippets in our various subdirs, if you don't mind.

"local" is communicating that this is make code local to this directory.
".mk" is communicating that this is a small make fragment/module.  taken
together, they say this is a local make fragment/module for this dir.

Makefile.inc is a bad name and deviates from what all other GNU/toolchain
projects are adopting.  i don't think newlib should be "innovating" here.

that said, i'm not the newlib maintainer, and if the newlib maintainer(s)
want to make "Makefile.inc" a hard requirement and not approve commits
using "local.mk", then that's the answer.  i disagree, but i don't get
the final say.
-mike
signature.asc (application/pgp-signature, 833 B)