Hi!

Thanks for the updated patches! I've gone over these and merged
several of them, the others I've left out for now, as I wanted to
get the current release out, which has been dragging for too long
already while I was finalizing it.

On Sun, 2022-02-13 at 18:38:19 +0100, Nicolas Boulenguez wrote:

> >From 5852b310ea8cdd519a0f7d6e1099c3c54db026ed Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Mon, 29 Jul 2019 14:38:32 +0200
> Subject: [PATCH 01/10] scripts/mk: stop hard-coding dpkg_datadir
> 
> The Makefile snippets include each other from their common directory,
> but the path differ during tests and after installation.  Instead of
> rewriting the file with a hardcoded path, compute it within Make.

Ah, nice, I think I like the dynamically computed path, yes. Although
to avoid changing all pathname concatenation I changed dpkg_datadir to
«$(patsubst %/,%,$(dir $(lastword $(MAKEFILE_LIST))))». But given that
I think we might still need to substitute other things I've left this
one out for now.

> >From 94c84d34ff28d81f2fceef797fa8314d7b03fb23 Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Thu, 11 Feb 2021 15:36:15 +0100
> Subject: [PATCH 02/10] scripts/t: test SOURCE_DATE_EPOCH
> 
> Set SOURCE_DATE_EPOCH either from the environment or the Debian changelog.
> Check that the value is (re)exported.

Merged. Changed from calling date(1) into hardcoding the timestamp
though.

> >From 32c2fad6ef96479afcffc38b40f8b2e82d3c46c4 Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Thu, 11 Feb 2021 15:45:03 +0100
> Subject: [PATCH 03/10] scripts/t: slightly optimize hash traversals
> 
> Iterate on key/value pairs instead of iterating on keys then search
> for each value.

Merged. Changed the tools variable names as they were originally not a
very good fit.

> >From cb0d31dc92f61144150ad2b042a01987540e0ddf Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Thu, 11 Feb 2021 16:09:48 +0100
> Subject: [PATCH 04/10] scripts/t: use loops instead of repetitions, check
>  exports and overrides
> 
> Replace copied lines with Make loops.
> 
> Add tests: architecture variable override, buildflags set and export,
> buildtool override and export.

I've left this one out for now.

> >From d27c9abc9d88a76c98597ee872adefd7c2dedd6a Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Thu, 11 Feb 2021 16:34:23 +0100
> Subject: [PATCH 05/10] scripts/buildtools.mk: remove unneeded conditionals
> 
> The ?= had no effect when the previous test was succeeding.  Make that
> explicit with an 'else'.
> 
> The 'ifdef' was always succeeding because previous stanza sets $1.

Merged.

> >From 26df5b04bb981bf9f1a23bf2341f5de1854e5daa Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nicolas.bouleng...@free.fr>
> Date: Sat, 13 Feb 2021 09:58:27 +0100
> Subject: [PATCH 06/10] scripts/buildtools.mk: indent for readability

Merged.

> >From cb1a48beaa613b7f55dee0842afbd5ba51495b74 Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Mon, 1 Nov 2021 10:08:08 +0100
> Subject: [PATCH 07/10] scripts/mk/buildopts.mk: small optimisations
> 
> Assign DEB_BUILD_OPTION_PARALLEL with := so that the value is computed
> only once instead of every time the variable is used.
> The maintainer is not supposed to modify DEB_BUILD_OPTIONS.
> 
> Always define DEB_BUILD_OPTION_PARALLEL, even if empty when
> DEB_BUILD_OPTIONS does not contain parallel=%.
> The distinction between DEB_BUILD_OPTIONS= and
> DEB_BUILD_OPTIONS=parallel= does probably not deserve a test.

I've left this one out as it is kind of an API change. I think it
might perhaps make more sense to fallback to setting it to 1 if it's
missing, but I need to ponder about possible consequences/fallout, etc.

> >From 4d63491c8dc9c6df85f0472d00f34c82e91ec05e Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Thu, 11 Feb 2021 16:26:50 +0100
> Subject: [PATCH 08/10] scripts/mk: reduce the number of subprocesses
> 
> architecture.mk and buildflags.mk spawn less subshells, improving the
> overall speed (the tests run twice faster according to bash time
> builtin).
> 
> pkg-info.mk uses the same trick than buildflags.mk in order to spawn
> at most one subshell. The performance gain is visible, but minor
> because there are way less variables.

I've left this one out for now. I'm not entirely satisfied with the
sed usage here. If we keep using sed, then I think it needs to be set
via a SED variable, substituted from the value found at configure
time. But then, I've been pondering whether we can have better export
formats, that might make the sed usage not necessary. I started with a
make-eval export mode for buildflags, but perhaps it would be better a
more generic formatting mode where the caller can specify how the
output should look like, akin «dpkg-query --showformat». Will ponder
about this.

> >From c92fd3aac8703475913db041c0bea53221757b5f Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Sun, 13 Feb 2022 14:17:20 +0100
> Subject: [PATCH 09/10] scripts/mk: protect against repeated inclusion
> 
> For example, buildtools.mk implicitly includes architecture.mk.

I've left this one out of now. I think it would be better to use a
dedicated variable instead of reusing the existing ones, in case the
callers set them for some reason, f.ex.

> >From 7a873a2e15dbb003aa9974c9019a7ca1821062e7 Mon Sep 17 00:00:00 2001
> From: Nicolas Boulenguez <nico...@debian.org>
> Date: Sun, 13 Feb 2022 13:41:26 +0100
> Subject: [PATCH 10/10] scripts/mk: improve details in documentation
> 
> architecture.mk: give more details
> Mention default.mk as an alternative in included scripts.
> Improve consistency accross Makefile snippets.
> Stop documenting version restrictions older than oldoldstable.

I've left this one out for now. The version information was added on
purpose to help people know when each interface or file was added, so
aid with dependency management or deciding when API usage might be
satisfactory.

Thanks,
Guillem

Reply via email to