bug-make
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug #65533] gmake-4.4.1 has a performance regression: at least the


From: DSB
Subject: Re: [bug #65533] gmake-4.4.1 has a performance regression: at least the nwchem project now builds much slower
Date: Sat, 30 Mar 2024 12:32:31 -0400

 > ... remove all those .EXPORT_ALL_VARIABLES: settings.  This is just a very bad idea. 

Sorry for the side-track but in my experience exporting *any* make variable is a bad idea. Where I work we maintain a "Make Bible" which says something to the effect that the only time it makes sense to export a make variable is if it's not _really_ a make variable. In other words this may be ok:

export PATH := $(PATH):/some/bin

Because $PATH isn't fundamentally a make variable, just a normal environment variable we can manipulate in make syntax, but there's no excuse for ever exporting a make variable that isn't originally an env var. There are only two cases:

1. You want to pass it to a non-make process invoked from a recipe. This falls into the exception above: it's not primarily a make variable or it wouldn't be useful to the non-make process.

2. You want to pass it to a recursively-invoked make. In this case it's far better to do so explicitly:

        $(MAKE) FOO=bar ...

Both because it's more self-documenting and because it narrows the scope. FOO will end up in the environment anyway since make automatically exports command line overrides but only in that branch of the process tree.

An environment variable is the logical equivalent of a global variable in C and suffers from all the same issues. Once you've exported a variable you've lost control over who comes to depend on it so it becomes exponentially harder to clean up the environment later. A good programmer limits the number of global variables and no competent programmer would make everything global. Same with the environment.

I understand these capabilities must be provided for POSIX and backward compatibility but I think they should be deprecated. I might go so far as to say so in the manual.

David

On Fri, Mar 29, 2024 at 1:54 PM Paul D. Smith <INVALID.NOREPLY@gnu.org> wrote:
Follow-up Comment #4, bug #65533 (group make):

I cloned the nwchem git repository and my suspicions were correct.

I see many, many instances of recursive variable assignment using $(shell ...)
syntax, such as:

src/nwc_columbus/sifs/GNUmakefile
38:ALLF = $(filter-out ./sifs_stubs.F, $(shell find . -name '*.F'))

(this is just one example there are 300+ spread across the makefiles).

At the same time I see that various makefiles are forcing all variable
assignments to be exported; for example:

src/peigs/src/c/Makefile.proto:.EXPORT_ALL_VARIABLES:


There are also instances of .EXPORT_ALL_VARIABLES in some Makefiles, but these
are probably not a problem because the Makefiles appear to be for BSD make
(GNUmakefile is used for GNU Make).  So, there appear to be about 5 makefiles
that have this setting.

But, when the Makefile.proto above is included in another makefile, that sets
that parameter for every make instance that includes it.

The simplest solution here is to go through and remove all those
.EXPORT_ALL_VARIABLES: settings.  This is just a very bad idea.  If there are
specific variables that need to be exported then those, and only those, should
be exported.

Unfortunately there is not any simple solution right now, except either fix
the makefiles or else limit your version of GNU Make to 4.3 or earlier before
this change was made.

I have been thinking about how to mitigate this issue and I just don't have
any good ideas.  There doesn't seem to be any good heuristic we can use to
avoid the behavior without also introducing bizarre and unexplainable edge
cases.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?65533>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]