git/shared.mak, branch v2.51.2

GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE

2024-12-20T20:36:45Z

Same as with the preceding commit, neither GIT_BUILT_FROM_COMMIT nor GIT_DATE can be overridden via the environment. Especially the latter is of importance given that we set it in our own "Documentation/doc-diff" script. Make the values of both variables overridable. Luckily we don't pull in these values via any included Makefiles, so the fix is trivial compared to the fix for GIT_VERSON. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

GIT-VERSION-GEN: fix overriding GIT_VERSION

2024-12-20T20:36:45Z

GIT-VERSION-GEN tries to derive the version that Git is being built from via multiple different sources in the following order: 1. A file called "version" in the source tree's root directory, if it exists. 2. The current commit in case Git is built from a Git repository. 3. Otherwise, we use a fallback version stored in a variable which is bumped whenever a new Git version is getting tagged. It used to be possible to override the version by overriding the `GIT_VERSION` Makefile variable (e.g. `make GIT_VERSION=foo`). This worked somewhat by chance, only: `GIT-VERSION-GEN` would write the actual Git version into `GIT-VERSION-FILE`, not the overridden value, but when including the file into our Makefile we would not override the `GIT_VERSION` variable because it has already been set by the user. And because our Makefile used the variable to propagate the version to our build tools instead of using `GIT-VERSION-FILE` the resulting build artifacts used the overridden version. But that subtle mechanism broke with 4838deab65 (Makefile: refactor GIT-VERSION-GEN to be reusable, 2024-12-06) and subsequent commits because the version information is not propagated via the Makefile variable anymore, but instead via the files that `GIT-VERSION-GEN` started to write. And as the script never knew about the `GIT_VERSION` environment variable in the first place it uses one of the values listed above instead of the overridden value. Fix this issue by making `GIT-VERSION-GEN` handle the case where `GIT_VERSION` has been set via the environment. Note that this requires us to introduce a new GIT_VERSION_OVERRIDE variable that stores a potential user-provided value, either via the environment or via "config.mak". Ideally we wouldn't need it and could just continue to use GIT_VERSION for this. But unfortunately, Makefiles will first include all sub-Makefiles before figuring out whether it needs to re-make any of them [1]. Consequently, if there already is a GIT-VERSION-FILE, we would have slurped in its value of GIT_VERSION before we call GIT-VERSION-GEN, and because GIT-VERSION-GEN now uses that value as an override it would mean that the first generated value for GIT_VERSION will remain unchanged. Furthermore we have to move the include for "GIT-VERSION-FILE" after the includes for "config.mak" and related so that GIT_VERSION_OVERRIDE can be set to the value provided by "config.mak". [1]: https://www.gnu.org/software/make/manual/html_node/Remaking-Makefiles.html Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Makefile: introduce template for GIT-VERSION-GEN

2024-12-20T20:36:45Z

Introduce a new template to call GIT-VERSION-GEN. This will allow us to iterate on how exactly the script is called in subsequent commits without having to adapt all call sites every time. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Makefile: simplify output of the libpath_template

2024-01-31T22:43:00Z

If a platform lacks the support to specify the dynamic library path, there is no suitable value to give to the CC_LD_DYNPATH variable. Allow them to be set to an empty string to signal that they do not need to add the usual -Wl,-rpath, or -R or whatever option followed by a directory name. This way, $(call libpath_template,$(SOMELIBDIR)) would expand to just a single mention of that directory, i.e. -L$(SOMELIBDIR) when CC_LD_DYNPATH is set to an empty string (or a "-L", which would have repeated the same "-L$(SOMELIBDIR)" twice without any ill effect). Signed-off-by: Junio C Hamano

Makefile: reduce repetitive library paths

2024-01-31T18:01:56Z

When we take a library package we depend on (e.g., LIBPCRE) from a directory other than the default location of the system, we add the same directory twice on the linker command like, like so: EXTLIBS += -L$(LIBPCREDIR)/$(lib) $(CC_LD_DYNPATH)$(LIBPCREDIR)/$(lib) Introduce a template "libpath_template" that takes the path to the directory, which can be used like so: EXTLIBS += $(call libpath_template,$(LIBPCREDIR)/$(lib)) and expand it into the "-L$(DIR) $(CC_LD_DYNPATH)$(DIR)" form. Hopefully we can reduce the chance of typoes this way. Signed-off-by: Junio C Hamano

Merge branch 'ab/gnumake-4.4-fix'

2022-12-01T09:38:07Z

Adjust our Makefiles for GNUmake 4.4 * ab/gnumake-4.4-fix: Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4

Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4

2022-11-30T22:24:12Z

Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has changed in a backward-incompatible way, as its "NEWS" file notes: Previously only simple (one-letter) options were added to the MAKEFLAGS variable that was visible while parsing makefiles. Now, all options are available in MAKEFLAGS. If you want to check MAKEFLAGS for a one-letter option, expanding "$(firstword -$(MAKEFLAGS))" is a reliable way to return the set of one-letter options which can be examined via findstring, etc. This upstream change meant that e.g.: make man Would become very noisy, because in shared.mak we rely on extracting "s" from the $(MAKEFLAGS), which now contains long options like "--jobserver-auth=fifo:", which we'll conflate with the "-s" option. So, let's change this idiom we've been carrying since [1], [2] and [3] as the "NEWS" suggests. Note that the "-" in "-$(MAKEFLAGS)" is critical here, as the variable will always contain leading whitespace if there are no short options, but long options are present. Without it e.g. "make --debug=all" would yield "--debug=all" as the first word, but with it we'll get "-" as intended. Then "-s" for "-s", "-Bs" for "-s -B" etc. 1. 0c3b4aac8ec (git-gui: Support of "make -s" in: do not output anything of the build itself, 2007-03-07) 2. b777434383b (Support of "make -s": do not output anything of the build itself, 2007-03-07) 3. bb2300976ba (Documentation/Makefile: make most operations "quiet", 2009-03-27) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

Makefile: don't create a ".build/.build/" for cocci, fix output

2022-11-11T22:21:45Z

Fix a couple of issues in the recently merged 0f3c55d4c2b (Merge branch 'ab/coccicheck-incremental' into next, 2022-11-08): In copying over the "contrib/coccinelle/" rules to ".build/contrib/coccinelle/" we inadvertently ended up with a ".build/.build/contrib/coccinelle/" as well. We'd generate the per-file patches in the former, and keep the rule and overall result in the latter. E.g. running: make contrib/coccinelle/free.cocci.patch COCCI_SOURCES="attr.c grep.c" Would, per "tree -a .build" yield the following result: .build ├── .build │ └── contrib │ └── coccinelle │ └── free.cocci.patch │ ├── attr.c │ ├── attr.c.log │ ├── grep.c │ └── grep.c.log └── contrib └── coccinelle ├── FOUND_H_SOURCES ├── free.cocci └── free.cocci.patch Now we'll instead generate all of our files in ".build/contrib/coccinelle/". Fixing this required renaming the directory where we keep our per-file patches, as we'd otherwise conflict with the result. Now the per-file patch directory is named e.g. "free.cocci.d". And the end result will now be: .build └── contrib └── coccinelle ├── FOUND_H_SOURCES ├── free.cocci ├── free.cocci.d │ ├── attr.c.patch │ ├── attr.c.patch.log │ ├── grep.c.patch │ └── grep.c.patch.log └── free.cocci.patch The per-file patches now have a ".patch" file suffix, which fixes another issue reported against 0f3c55d4c2b: The summary output was confusing. Before for the "make" command above we'd emit: [...] MKDIR -p .build/contrib/coccinelle CP contrib/coccinelle/free.cocci .build/contrib/coccinelle/free.cocci GEN .build/contrib/coccinelle/FOUND_H_SOURCES MKDIR -p .build/.build/contrib/coccinelle/free.cocci.patch SPATCH .build/.build/contrib/coccinelle/free.cocci.patch/grep.c SPATCH .build/.build/contrib/coccinelle/free.cocci.patch/attr.c SPATCH CAT $^ >.build/contrib/coccinelle/free.cocci.patch CP .build/contrib/coccinelle/free.cocci.patch contrib/coccinelle/free.cocci.patch But now we'll instead emit (identical output at the start omitted): [...] MKDIR -p .build/contrib/coccinelle/free.cocci.d SPATCH grep.c >.build/contrib/coccinelle/free.cocci.d/grep.c.patch SPATCH attr.c >.build/contrib/coccinelle/free.cocci.d/attr.c.patch SPATCH CAT .build/contrib/coccinelle/free.cocci.d/**.patch >.build/contrib/coccinelle/free.cocci.patch CP .build/contrib/coccinelle/free.cocci.patch contrib/coccinelle/free.cocci.patch I.e. we have an "SPATCH" line that makes it clear that we're running against the "{attr,grep}.c" file. The "SPATCH CAT" is then altered to correspond to it, showing that we're concatenating the "free.cocci.d/**.patch" files into one generated "free.cocci.patch" at the end. Reported-by: SZEDER Gábor Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau

Makefile: copy contrib/coccinelle/*.cocci to build/

2022-11-03T01:22:16Z

Change the "coccinelle" rule so that we first copy the *.cocci source in e.g. "contrib/coccinelle/strbuf.cocci" to ".build/contrib/coccinelle/strbuf.cocci" before operating on it. For now this serves as a rather pointless indirection, but prepares us for the subsequent commit where we'll be able to inject generated *.cocci files. Having the entire dependency tree live inside .build/* simplifies both the globbing we'd need to do, and any "clean" rules. It will also help for future targets which will want to act on the generated patches or the logs, e.g. targets to alert if we can't parse certain files (or, less so than usual) with "spatch", and e.g. a replacement for "ci/run-static-analysis.sh". Such a replacement won't care about placing the patches in the in-tree, only whether they're "OK" (and about the diff). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau

cocci: make "coccicheck" rule incremental

2022-11-03T01:22:16Z

Optimize the very slow "coccicheck" target to take advantage of incremental rebuilding, and fix outstanding dependency problems with the existing rule. The rule is now faster both on the initial run as we can make better use of GNU make's parallelism than the old ad-hoc combination of make's parallelism combined with $(SPATCH_BATCH_SIZE) and/or the "--jobs" argument to "spatch(1)". It also makes us *much* faster when incrementally building, it's now viable to "make coccicheck" as topic branches are merged down. The rule didn't use FORCE (or its equivalents) before, so a: make coccicheck make coccicheck Would report nothing to do on the second iteration. But all of our patch output depended on all $(COCCI_SOURCES) files, therefore e.g.: make -W grep.c coccicheck Would do a full re-run, i.e. a a change in a single file would force us to do a full re-run. The reason for this (not the initial rationale, but my analysis) is: * Since we create a single "*.cocci.patch+" we don't know where to pick up where we left off, or how to incrementally merge e.g. a "grep.c" change with an existing *.cocci.patch. * We've been carrying forward the dependency on the *.c files since 63f0a758a06 (add coccicheck make target, 2016-09-15) the rule was initially added as a sort of poor man's dependency discovery. As we don't include other *.c files depending on other *.c files has always been broken, as could be trivially demonstrated e.g. with: make coccicheck make -W strbuf.h coccicheck However, depending on the corresponding *.c files has been doing something, namely that *if* an API change modified both *.c and *.h files we'd catch the change to the *.h we care about via the *.c being changed. For API changes that happened only via *.h files we'd do the wrong thing before this change, but e.g. for function additions (not "static inline" ones) catch the *.h change by proxy. Now we'll instead: * Create a / pair in the .build directory, E.g. for swap.cocci and grep.c we'll create .build/contrib/coccinelle/swap.cocci.patch/grep.c. That file is the diff we'll apply for that - combination, if there's no changes to me made (the common case) it'll be an empty file. * Our generated *.patch file (e.g. contrib/coccinelle/swap.cocci.patch) is now a simple "cat $^" of all of all of the / files for a given . In the case discussed above of "grep.c" being changed we'll do the full "cat" every time, so they resulting *.cocci.patch will always be correct and up-to-date, even if it's "incrementally updated". See 1cc0425a27c (Makefile: have "make pot" not "reset --hard", 2022-05-26) for another recent rule that used that technique. As before we'll: * End up generating a contrib/coccinelle/swap.cocci.patch, if we "fail" by creating a non-empty patch we'll still exit with a zero exit code. Arguably we should move to a more Makefile-native way of doing this, i.e. fail early, and if we want all of the "failed" changes we can use "make -k", but as the current "ci/run-static-analysis.sh" expects us to behave this way let's keep the existing behavior of exhaustively discovering all cocci changes, and only failing if spatch itself errors out. Further implementation details & notes: * Before this change running "make coccicheck" would by default end up pegging just one CPU at the very end for a while, usually as we'd finish whichever *.cocci rule was the most expensive. This could be mitigated by combining "make -jN" with SPATCH_BATCH_SIZE, see 960154b9c17 (coccicheck: optionally batch spatch invocations, 2019-05-06). There will be cases where getting rid of "SPATCH_BATCH_SIZE" makes things worse, but a from-scratch "make coccicheck" with the default of SPATCH_BATCH_SIZE=1 (and tweaking it doesn't make a difference) is faster (~3m36s v.s. ~3m56s) with this approach, as we can feed the CPU more work in a less staggered way. * Getting rid of "SPATCH_BATCH_SIZE" particularly helps in cases where the default of 1 yields parallelism under "make coccicheck", but then running e.g.: make -W contrib/coccinelle/swap.cocci coccicheck I.e. before that would use only one CPU core, until the user remembered to adjust "SPATCH_BATCH_SIZE" differently than the setting that makes sense when doing a non-incremental run of "make coccicheck". * Before the "make coccicheck" rule would have to clean "contrib/coccinelle/*.cocci.patch*", since we'd create "*+" and "*.log" files there. Now those are created in .build/contrib/coccinelle/, which is covered by the "cocciclean" rule already. Outstanding issues & future work: * We could get rid of "--all-includes" in favor of manually specifying a list of includes to give to "spatch(1)". As noted upthread of [1] a naïve removal of "--all-includes" will result in broken *.cocci patches, but if we know the exhaustive list of includes via COMPUTE_HEADER_DEPENDENCIES we don't need to re-scan for them, we could grab the headers to include from the .depend.d/.o.d and supply them with the "--include" option to spatch(1).q 1. https://lore.kernel.org/git/87ft18tcog.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau