• Bug#872381: dpkg-dev: optimize Makefile snippets for debian/rules

    From Nicolas Boulenguez@1:229/2 to All on Tue May 7 12:40:02 2024
    XPost: linux.debian.bugs.dist
    From: nicolas@debian.org

    Package: dpkg-dev
    Followup-For: Bug #872381

    Hello.
    It is good to see the main suggestion merged. Thanks!

    You have not applied
    0001-scripts-mk-stop-hard-coding-dpkg_datadir.patch
    probably because you prefer the related parts in
    f1175056 (build: Rework subst handling for built or installed artifacts).

    Ironically, f1175056 seems to introduce the exact kind of human error
    that dynamic generation would prevent. 0001-build-spare-an-unneeded-subst-handling-in-pkg-info.m.patch

    From 36e98fdd10b1896f8fa89733b5e0c1781c0cce4c Mon Sep 17 00:00:00 2001
    From: Nicolas Boulenguez <nicolas@debian.org>
    Date: Mon, 6 May 2024 10:52:49 +0200
    Subject: [PATCH] build: spare an unneeded subst handling in pkg-info.mk

    This commits follows f1175056.
    ---
    scripts/mk/Makefile.am | 1 -
    1 file changed, 1 deletion(-)

    diff --git a/scripts/mk/Makefile.am b/scripts/mk/Makefile.am
    index be6076b2c..5f086ef49 100644
    --- a/scripts/mk/Makefile.am
    +++ b/scripts/mk/Makefile.am
    @@ -18,5 +18,4 @@ include $(top_srcdir)/build-aux/subst.am
    install-data-hook:
    $(subst_make_file) $(DESTDIR)$(pkgdatadir)/default.mk
    $(subst_make_file) $(DESTDIR)$(pkgdatadir)/buildtools.mk
    - $(subst_make_file) $(DESTDIR)$(pkgdatadir)/pkg-info.mk
    $(subst_make_file) $(DESTDIR)$(pkgdatadir)/vendor.mk
    --
    2.39.2


    From 7daa3aca068d997c6895757cb58ba91d66bd6842 Mon Sep 17 00:00:00 2001
    From: Nicolas Boulenguez <nicolas@debian.org>
    Date: Mon, 6 May 2024 11:37:14 +0200
    Subject: [PATCH] scripts/mk: stop hard-coding dpkg_datadir

    This path differ during tests and after installation. Instead of
    rewriting the file with a hardcoded path, compute it within Make.
    ---
    build-aux/subst.am | 8 --------
    scripts/mk/Makefile.am | 10 ----------
    scripts/mk/buildtools.mk |
  • From Nicolas Boulenguez@1:229/2 to All on Mon Jun 24 20:00:01 2024
    XPost: linux.debian.bugs.dist
    From: nicolas@debian.org

    Package: dpkg-dev
    Followup-For: Bug #872381

    Hello again.
    My last message was confusing.

    I am suggesting to improve commit f1175056 with 0001-build-spare-an-unneeded-subst-handling-in-pkg-info.m.patch.

    I have only quoted 0001-scripts-mk-stop-hard-coding-dpkg_datadir.patch
    for context. Please ignore it.

    --- SoupGate-Win32 v1.05
    * Origin: you cannot sedate... all the things you hate (1:229/2)
  • From Guillem Jover@1:229/2 to Guillem Jover on Mon Jul 1 06:10:01 2024
    XPost: linux.debian.bugs.dist
    From: guillem@debian.org

    Hi!

    On Mon, 2024-07-01 at 04:43:40 +0200, Guillem Jover wrote:
    On Sat, 2024-06-29 at 17:29:49 +0200, Guillem Jover wrote:
    There's another regression that I found on various BSDs, where they
    already have GNU make 4.4.1, where the buildtools test fails for the
    AR and AR_FOR_BUILD checks, and I've not managed to track down yet
    what the problem is (and whether this is an intentional behavior
    change from its NEWS entry), but this needs to be fixed before a release because Debian will eventually hit that, and other systems with a new
    GNU make will also fail. If you could have a look that would be appreciated, otherwise I'll try during the weekend.

    I've tracked this down. The problem was that on the BSDs the dpkg
    build system sets CC as the value detected at configure time, which is
    cc, and for which there was no mocked binary, so the test code ended
    up running as if it was cross-building, which cause the value to not
    inherit from AR. I'll be adding a mock for cc, but that will not catch
    CC being set to clang or another program, so this needs to be dynamic,
    and so for now I'll explicitly override AR_FOR_BUILD to avoid the cross-compilation check.

    Actually simply overriding the CC to gcc in the mk.t does it and makes
    it into a controlled environment.

    (I'm also going to add some CI runs for the BSDs to avoid regressions,
    but probably on a branch for github.)

    Thanks,
    Guillem

    --- SoupGate-Win32 v1.05
    * Origin: you cannot sedate... all the things you hate (1:229/2)
  • From Guillem Jover@1:229/2 to Guillem Jover on Mon Jul 1 04:50:01 2024
    XPost: linux.debian.bugs.dist
    From: guillem@debian.org

    Hi!

    On Sat, 2024-06-29 at 17:29:49 +0200, Guillem Jover wrote:
    There's another regression that I found on various BSDs, where they
    already have GNU make 4.4.1, where the buildtools test fails for the
    AR and AR_FOR_BUILD checks, and I've not managed to track down yet
    what the problem is (and whether this is an intentional behavior
    change from its NEWS entry), but this needs to be fixed before a release because Debian will eventually hit that, and other systems with a new
    GNU make will also fail. If you could have a look that would be
    appreciated, otherwise I'll try during the weekend.

    I've tracked this down. The problem was that on the BSDs the dpkg
    build system sets CC as the value detected at configure time, which is
    cc, and for which there was no mocked binary, so the test code ended
    up running as if it was cross-building, which cause the value to not
    inherit from AR. I'll be adding a mock for cc, but that will not catch
    CC being set to clang or another program, so this needs to be dynamic,
    and so for now I'll explicitly override AR_FOR_BUILD to avoid the cross-compilation check.

    (I'm also going to add some CI runs for the BSDs to avoid regressions,
    but probably on a branch for github.)

    Thanks,
    Guillem

    --- SoupGate-Win32 v1.05
    * Origin: you cannot sedate... all the things you hate (1:229/2)
  • From Guillem Jover@1:229/2 to Nicolas Boulenguez on Sat Jun 29 17:40:01 2024
    XPost: linux.debian.bugs.dist
    From: guillem@debian.org

    Hi!

    On Mon, 2024-05-06 at 12:32:45 +0200, Nicolas Boulenguez wrote:
    You have not applied
    0001-scripts-mk-stop-hard-coding-dpkg_datadir.patch
    probably because you prefer the related parts in
    f1175056 (build: Rework subst handling for built or installed artifacts).

    As I think I mentioned at the time I think we need to parametrize sed
    anyway, and as we have to subst it, removing the support from the
    build system would go counter to that.

    Ironically, f1175056 seems to introduce the exact kind of human error
    that dynamic generation would prevent. 0001-build-spare-an-unneeded-subst-handling-in-pkg-info.m.patch

    diff --git a/scripts/mk/Makefile.am b/scripts/mk/Makefile.am
    index be6076b2c..5f086ef49 100644
    --- a/scripts/mk/Makefile.am
    +++ b/scripts/mk/Makefile.am
    @@ -18,5 +18,4 @@ include $(top_srcdir)/build-aux/subst.am
    install-data-hook:
    $(subst_make_file) $(DESTDIR)$(pkgdatadir)/default.mk
    $(subst_make_file) $(DESTDIR)$(pkgdatadir)/buildtools.mk
    - $(subst_make_file) $(DESTDIR)$(pkgdatadir)/pkg-info.mk
    $(subst_make_file) $(DESTDIR)$(pkgdatadir)/vendor.mk

    Ah, nice catch thanks, that was actually a problem with a botched
    rebase for the parametrized sed, but instead I was planning on
    queueing:

    https://git.hadrons.org/cgit/debian/dpkg/dpkg.git/commit/?h=pu/build-sys-subst-sed&id=56a6de1eb823e4e9bea0c7605f326c86f601ea05

    Because otherwise running the test suite on BSDs with the base sed tool,
    makes them fail on the pkg-info.mk.

    There's another regression that I found on various BSDs, where they
    already have GNU make 4.4.1, where the buildtools test fails for the
    AR and AR_FOR_BUILD checks, and I've not managed to track down yet
    what the problem is (and whether this is an intentional behavior
    change from its NEWS entry), but this needs to be fixed before a release because Debian will eventually hit that, and other systems with a new
    GNU make will also fail. If you could have a look that would be
    appreciated, otherwise I'll try during the weekend.

    Thanks,
    Guillem

    --- SoupGate-Win32 v1.05
    * Origin: you cannot sedate... all the things you hate (1:229/2)
  • From Nicolas Boulenguez@1:229/2 to All on Tue Jul 2 14:00:01 2024
    XPost: linux.debian.bugs.dist
    From: nicolas@debian.org

    Summary of inadvertently private mails:
    mk-bsd-sed-quotation-mark.diff hopefully fixes the build on BSD.

    Assuming this, and that I am not missing the point again, mk-unhardcode-datadir.diff can be applied again.
    The attachment improves its readability.

    * It replaces "ifndef ... := ... endif" with "?=".
    The variable will be searched for $ on each expansion, but the
    performance difference is probably quite small.

    * It removes the comment explaining where dpkg_datadir might already
    be defined.
    Anyone working on this will know about default.mk.

    * It replaces $(patsubst ...) with $(dir ...).
    This was the original suggestion.
    You once answered:
    to avoid changing all pathname concatenation I changed dpkg_datadir to «$(patsubst %/,%,$(dir $(lastword $(MAKEFILE_LIST))))».
    then I did suggest $(patsubst %/current.mk/,%,$(lastword $(MAKEFILE_LIST))),
    but, thinking about it again, there seems to be no problem with a
    double path component separator.

    From 467d7aa41c95796f6d7ef43bb4c8fda925728791 Mon Sep 17 00:00:00 2001
    From: Nicolas Boulenguez <nicolas@debian.org>
    Date: Tue, 2 Jul 2024 11:54:55 +0200
    Subject: scripts/mk: Stop hard-coding dpkg_datadir

    This path differ during tests and after installation. Instead of
    rewriting the file with a hardcoded path, compute it within Make.

    diff --git a/build-aux/subst.am b/build-aux/subst.am
    index 7785e4af7..9c96e5ce0 100644
    --- a/build-aux/subst.am
    +++ b/build-aux/subst.am
    @@ -45,11 +45,3 @@ SUFFIXES += .pl
    @test -d `dirname $@` || $(MKDIR_P) `dirname $@`
    $(AM_V_GEN) $(subst_perl_filter) <$< >$@
    $(AM_V_at) chmod +x $@
    -
    -# Makefile support.
    -
    -subst_make_rules = "\
    - s{dpkg_datadir\s*=\s*[^\s]*}{dpkg_datadir = $(pkgdatadir)}; \
    - "
    -
    -subst_make_file = $(PERL) -i -p -e $(subst_make_rules)
    diff --git a/scripts/mk/Makefile.am b/scripts/mk/Makefile.am
    index be6076b2c..6e85e17b9 100644
    --- a/scripts/mk/Makefile.am
    +++ b/scripts/mk/Makefile.am
    @@ -10,13 +10,3 @@ dist_pkgdata_DATA = \
    pkg-info.mk \
    vendor.mk \
    # EOL
    -
    -SUFFIXES =
    -
    -include $(top_srcdir)/build-aux/subst.am
    -
    -install-data-hook:
    - $(subst_make_file) $(DESTDIR)$(pkgdatadir)/default.mk
    - $(subst_make_file) $(DESTDIR)$(pkgdatadir)/buildtools.mk
    - $(subst_make_file) $(DESTDIR)$(pkgdatadir)/pkg-info.mk
    - $(subst_make_file) $(DESTDIR)$(pkgdatadir)/vendor.mk
    diff --git a/scripts/mk/buildtools.mk b/scripts/mk/buildtools.mk
    index 6ce