From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH] SKIP_DASHED_BUILT_INS: respect `config.mak`
Date: Fri, 22 Jan 2021 22:16:52 +0100 (CET) [thread overview]
Message-ID: <nycvar.QRO.7.76.6.2101221935230.52@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <xmqqft2tc2w7.fsf@gitster.c.googlers.com>
Hi Junio,
On Thu, 21 Jan 2021, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > When `SKIP_DASHED_BUILT_INS` is specified in `config.mak`, the dashed
> > form of the built-ins was still generated.
> >
> > By moving the `SKIP_DASHED_BUILT_INS` handling after `config.mak` was
> > read, this can be avoided.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
>
> OK. So the problem is that the moved block that sets ALL_PROGRAMS,
> ALL_COMMANDS_TO_INSTALL, etc. depends on $(SKIP_DASHED_BUILT_INS),
> and that happens before we "include config.mak".
That matches my understanding, yes.
> That makes sense. Will apply (I do not know if you want this also
> on the maint tracks and if so which ones---I think it would matter
> if you want to cut a maint release from 2.29.x or 2.30.x tracks).
>
> By the way, I wonder if we can (semi-)automate looking for such a
> mistake in the future. Does a simple rule like:
>
> No variable that has "Define X if you want to distim the doshes"
> at the beginning of the Makefile must be referenced before we
> include config.mak
>
> work?
That would work, but we do not have a consistent format there. Exhibit A:
# When using RUNTIME_PREFIX, define HAVE_BSD_KERN_PROC_SYSCTL if your platform
# supports the KERN_PROC BSD sysctl function.
Therefore, automating this check would probably be a bit of a challenge.
The best I could come up with (and which is still not complete) within few
dozen minutes was this:
terms=$(sed -ne '/^#$/{N;s/^#\n# [^ ].*\? \([A-Z][A-Z0-9_]\{4,\}\) .*/\1\\|/p}' -e '/GIT-VERSION-FILE/q' Makefile)
sed -n "/^GIT-VERSION-FILE:/,/^-include config.mak/s/\\($(echo "$terms" | tr -d '\012')NO-MATCH\\)/&/p" Makefile
The only thing that sticks out in this output is that we use SHELL_PATH a
couple times before including config.mak.
And I don't think that this hack of mine can be converted into a robust
check that we'd want to run to verify that the Makefile does not use
constants before they are potentially defined in config.mak,
unfortunately.
Ciao,
Dscho
next prev parent reply other threads:[~2021-01-22 21:20 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-21 13:09 [PATCH] SKIP_DASHED_BUILT_INS: respect `config.mak` Johannes Schindelin via GitGitGadget
2021-01-21 22:59 ` Junio C Hamano
2021-01-22 21:16 ` Johannes Schindelin [this message]
2021-01-22 21:50 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=nycvar.QRO.7.76.6.2101221935230.52@tvgsbejvaqbjf.bet \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).