Hello.

> DEB_VERSION_UPSTREAM_REVISION not DEB_UPSTREAM_REVISION

Good catch!

> But it doesn't really matter anymore, once this part is removed.

The removed part implements lazy evaluation and does matter for
performances.  Thanks to your diagnostic, we may now restore it.
>From d713c8b47b16cee20e6bfe66aa5ba5b43d8129d6 Mon Sep 17 00:00:00 2001
From: Nicolas Boulenguez <nico...@debian.org>
Date: Thu, 25 Jul 2024 10:48:30 +0200
Subject: [PATCH 1/2] Revert "scripts/mk: Fix pkg-info.mk evaluation by adding
 new DEB_TIMESTAMP variable"

This reverts commit c05f82972018d182fa296ef38384a1adddf5b6eb.
---
 scripts/mk/pkg-info.mk   | 21 +++++++++++++--------
 scripts/t/mk/pkg-info.mk |  2 --
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/scripts/mk/pkg-info.mk b/scripts/mk/pkg-info.mk
index 37bf42299..ddda4f736 100644
--- a/scripts/mk/pkg-info.mk
+++ b/scripts/mk/pkg-info.mk
@@ -8,9 +8,6 @@
 #   DEB_VERSION_UPSTREAM: package's upstream version.
 #   DEB_DISTRIBUTION: distribution(s) listed in the current debian/changelog
 #     entry.
-#   DEB_TIMESTAMP: source package release date as seconds since the epoch as
-#     specified in the latest debian/changelog entry (since dpkg 1.22.9),
-#     although you are probably looking for SOURCE_DATE_EPOCH instead.
 #
 #   SOURCE_DATE_EPOCH: source release date as seconds since the epoch, as
 #     specified by <https://reproducible-builds.org/specs/source-date-epoch/>
@@ -29,12 +26,20 @@ dpkg_parsechangelog_run = $(eval $(shell dpkg-parsechangelog | sed -n '\
     $$(eval DEB_VERSION_EPOCH_UPSTREAM:=\1\2\4)\
     $$(eval DEB_VERSION_UPSTREAM_REVISION:=\2\3)\
     $$(eval DEB_VERSION_UPSTREAM:=\2\4)/p;\
-  s/^Timestamp: \(.*\)/$$(eval DEB_TIMESTAMP:=\1)/p'))
+  s/^Timestamp: \(.*\)/$$(eval SOURCE_DATE_EPOCH?=\1)/p'))
 
-# Compute all the values in one go.
-$(dpkg_parsechangelog_run)
-
-SOURCE_DATE_EPOCH ?= $(DEB_TIMESTAMP)
+ifdef SOURCE_DATE_EPOCH
+  dpkg_lazy_eval ?= $(eval $(1) = $(2)$$($(1)))
+  $(call dpkg_lazy_eval,DEB_DISTRIBUTION,$$(dpkg_parsechangelog_run))
+  $(call dpkg_lazy_eval,DEB_SOURCE,$$(dpkg_parsechangelog_run))
+  $(call dpkg_lazy_eval,DEB_VERSION,$$(dpkg_parsechangelog_run))
+  $(call dpkg_lazy_eval,DEB_VERSION_EPOCH_UPSTREAM,$$(dpkg_parsechangelog_run))
+  $(call dpkg_lazy_eval,DEB_VERSION_UPSTREAM,$$(dpkg_parsechangelog_run))
+  $(call dpkg_lazy_eval,DEB_UPSTREAM_REVISION,$$(dpkg_parsechangelog_run))
+else
+  # We need to compute the values right now.
+  $(dpkg_parsechangelog_run)
+endif
 export SOURCE_DATE_EPOCH
 
 endif # dpkg_pkg_info_mk_included
diff --git a/scripts/t/mk/pkg-info.mk b/scripts/t/mk/pkg-info.mk
index 6863ebb80..6eb37866d 100644
--- a/scripts/t/mk/pkg-info.mk
+++ b/scripts/t/mk/pkg-info.mk
@@ -6,7 +6,6 @@ TEST_DEB_VERSION_EPOCH_UPSTREAM = 1:2:3.4-5
 TEST_DEB_VERSION_UPSTREAM_REVISION = 2:3.4-5-6
 TEST_DEB_VERSION_UPSTREAM = 2:3.4-5
 TEST_DEB_DISTRIBUTION = suite
-TEST_DEB_TIMESTAMP = 1438697630
 
 test_vars := \
   DEB_SOURCE \
@@ -15,7 +14,6 @@ test_vars := \
   DEB_VERSION_UPSTREAM_REVISION \
   DEB_VERSION_UPSTREAM \
   DEB_DISTRIBUTION \
-  DEB_TIMESTAMP \
   SOURCE_DATE_EPOCH \
   # EOL
 
-- 
2.39.2

>From 6a334f57b6dd21a522258941fdd90d20c4d7197d Mon Sep 17 00:00:00 2001
From: Nicolas Boulenguez <nico...@debian.org>
Date: Thu, 25 Jul 2024 10:50:43 +0200
Subject: [PATCH 2/2] scripts/mk: fix #1076863 DEB_VERSION_UPSTREAM_REVISION
 but keep lazy expansion

In e146a68a, the variables (list A) are assigned with a lazy expansion
wrapper, so that first expansion triggers dpkg-parsechangelog and
actually assigns the variables (list B) with their final values.

A mispelling of DEB_VERSION_UPSTREAM_REVISION in (A) causes #1076863
and its clones (thanks to Michael Tokarev <m...@tls.msk.ru>).

The issue was not detected by tests and hard to understand because (B)
was correct, hiding the bug after any other expansion.  For example,
variables expansions within recipes were less likely to be affected
because they happen long after plain assignments.

Testing this for regression would require a separate test for each
variable and does not seem worth the while.

Commit c05f8297 fixes the bug but runs dpkg-parsechangelog each time a
debian/rules including pkg-info.mk is parsed.

This fix preserves lazy expansion.
---
 scripts/mk/pkg-info.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/mk/pkg-info.mk b/scripts/mk/pkg-info.mk
index ddda4f736..a51dd88e4 100644
--- a/scripts/mk/pkg-info.mk
+++ b/scripts/mk/pkg-info.mk
@@ -35,7 +35,7 @@ ifdef SOURCE_DATE_EPOCH
   $(call dpkg_lazy_eval,DEB_VERSION,$$(dpkg_parsechangelog_run))
   $(call dpkg_lazy_eval,DEB_VERSION_EPOCH_UPSTREAM,$$(dpkg_parsechangelog_run))
   $(call dpkg_lazy_eval,DEB_VERSION_UPSTREAM,$$(dpkg_parsechangelog_run))
-  $(call dpkg_lazy_eval,DEB_UPSTREAM_REVISION,$$(dpkg_parsechangelog_run))
+  $(call dpkg_lazy_eval,DEB_VERSION_UPSTREAM_REVISION,$$(dpkg_parsechangelog_run))
 else
   # We need to compute the values right now.
   $(dpkg_parsechangelog_run)
-- 
2.39.2

Reply via email to