[1/2] fix: lazy expand ISAR_APT_SNAPSHOT_DATE[flag]

Message ID 20251006141227.1017941-1-felix.moessbauer@siemens.com
State Accepted, archived
Headers show
Series [1/2] fix: lazy expand ISAR_APT_SNAPSHOT_DATE[flag] | expand

Commit Message

MOESSBAUER, Felix Oct. 6, 2025, 2:12 p.m. UTC
The ISAR_APT_SNAPSHOT_DATE variable controls the timestamp of the
sources on the snapshot mirror. This variable further is a shortcut for
ISAR_APT_SNAPSHOT_TIMESTAMP that avoids letting the user compute a unix
timestamp of the desired date. However, while
ISAR_APT_SNAPSHOT_TIMESTAMP is always available via bitbake conf, the
_DATE variant is only available after computing it in do_bootstrap.

In 394e954, support was added to just set the timestamp / date for the
security distribution by extending the _TIMESTAMP and _DATE variables
with a flag. However, the logic that evaluates the variable in
DISTRO_APT_SNAPSHOT_PREMIRROR is defined in the distro conf, which is
parsed earlier than the logic in do_bootstrap. By that, depending on the
context the variable might not be set leading to incorrect substitutions
of the apt-sources. This has not been observed in actual images, but in
the devshell and when printing the variable with bitbake.

Anyways, the value of the variable should not depend on the parsing
order. To fix this, we now do not expand the variable until using it.

Fixes: 394e9540 ("snapshots: add option to use separate timestamp ...")
Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
---
 meta/conf/distro/debian-common.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Florian Bezdeka Oct. 7, 2025, 10:39 a.m. UTC | #1
On Mon, 2025-10-06 at 16:12 +0200, 'Felix Moessbauer' via isar-users
wrote:
> The ISAR_APT_SNAPSHOT_DATE variable controls the timestamp of the
> sources on the snapshot mirror. This variable further is a shortcut for
> ISAR_APT_SNAPSHOT_TIMESTAMP that avoids letting the user compute a unix
> timestamp of the desired date. However, while
> ISAR_APT_SNAPSHOT_TIMESTAMP is always available via bitbake conf, the
> _DATE variant is only available after computing it in do_bootstrap.
> 
> In 394e954, support was added to just set the timestamp / date for the
> security distribution by extending the _TIMESTAMP and _DATE variables
  ^^^^^^^^^^^^^^^^^^^^
Wording: distribution -> repository?

According to https://wiki.debian.org/SourcesList this seems the right
term here.

> with a flag. However, the logic that evaluates the variable in
> DISTRO_APT_SNAPSHOT_PREMIRROR is defined in the distro conf, which is
> parsed earlier than the logic in do_bootstrap. By that, depending on the
> context the variable might not be set leading to incorrect substitutions
> of the apt-sources. This has not been observed in actual images, but in
> the devshell and when printing the variable with bitbake.
> 
> Anyways, the value of the variable should not depend on the parsing
> order. To fix this, we now do not expand the variable until using it.

Agree.

> 
> Fixes: 394e9540 ("snapshots: add option to use separate timestamp ...")
> Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> ---
>  meta/conf/distro/debian-common.conf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/conf/distro/debian-common.conf b/meta/conf/distro/debian-common.conf
> index 8da17d6f..92daddc3 100644
> --- a/meta/conf/distro/debian-common.conf
> +++ b/meta/conf/distro/debian-common.conf
> @@ -45,6 +45,6 @@ DISTRO_BOOTSTRAP_BASE_PACKAGES:append:bullseye = " usrmerge"
>  
>  # snapshot mirror for reproducible builds
>  DISTRO_APT_SNAPSHOT_PREMIRROR ??= " \
> -    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security')}\n \
> +    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security', expand=False)}\n \
>      deb.debian.org/(.*)/? snapshot.debian.org/archive/\1/${ISAR_APT_SNAPSHOT_DATE}\n \
>  "
> -- 
> 2.51.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/isar-users/20251006141227.1017941-1-felix.moessbauer%40siemens.com.
MOESSBAUER, Felix Oct. 7, 2025, 2:39 p.m. UTC | #2
On Tue, 2025-10-07 at 12:39 +0200, Florian Bezdeka wrote:
> On Mon, 2025-10-06 at 16:12 +0200, 'Felix Moessbauer' via isar-users
> wrote:
> > The ISAR_APT_SNAPSHOT_DATE variable controls the timestamp of the
> > sources on the snapshot mirror. This variable further is a shortcut for
> > ISAR_APT_SNAPSHOT_TIMESTAMP that avoids letting the user compute a unix
> > timestamp of the desired date. However, while
> > ISAR_APT_SNAPSHOT_TIMESTAMP is always available via bitbake conf, the
> > _DATE variant is only available after computing it in do_bootstrap.
> > 
> > In 394e954, support was added to just set the timestamp / date for the
> > security distribution by extending the _TIMESTAMP and _DATE variables
>   ^^^^^^^^^^^^^^^^^^^^
> Wording: distribution -> repository?
> 
> According to https://wiki.debian.org/SourcesList this seems the right
> term here.

But there they also state:

Apt downloads and installs packages from one or more software
repositories (sometimes called suites, distributions, versions, or even
sources). 

As we currently call this dist / distribution in the source code, I
prefer to also use that term in the commit message.

If a debian policy states the name, I would change it, but just because
it is mentioned in the wiki it not enough to require the change.

Felix

> 
> > with a flag. However, the logic that evaluates the variable in
> > DISTRO_APT_SNAPSHOT_PREMIRROR is defined in the distro conf, which is
> > parsed earlier than the logic in do_bootstrap. By that, depending on the
> > context the variable might not be set leading to incorrect substitutions
> > of the apt-sources. This has not been observed in actual images, but in
> > the devshell and when printing the variable with bitbake.
> > 
> > Anyways, the value of the variable should not depend on the parsing
> > order. To fix this, we now do not expand the variable until using it.
> 
> Agree.
> 
> > 
> > Fixes: 394e9540 ("snapshots: add option to use separate timestamp ...")
> > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > ---
> >  meta/conf/distro/debian-common.conf | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/conf/distro/debian-common.conf b/meta/conf/distro/debian-common.conf
> > index 8da17d6f..92daddc3 100644
> > --- a/meta/conf/distro/debian-common.conf
> > +++ b/meta/conf/distro/debian-common.conf
> > @@ -45,6 +45,6 @@ DISTRO_BOOTSTRAP_BASE_PACKAGES:append:bullseye = " usrmerge"
> >  
> >  # snapshot mirror for reproducible builds
> >  DISTRO_APT_SNAPSHOT_PREMIRROR ??= " \
> > -    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security')}\n \
> > +    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security', expand=False)}\n \
> >      deb.debian.org/(.*)/? snapshot.debian.org/archive/\1/${ISAR_APT_SNAPSHOT_DATE}\n \
> >  "
> > -- 
> > 2.51.0
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups "isar-users" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
> > To view this discussion visit https://groups.google.com/d/msgid/isar-users/20251006141227.1017941-1-felix.moessbauer%40siemens.com.
Florian Bezdeka Oct. 7, 2025, 2:57 p.m. UTC | #3
On Tue, 2025-10-07 at 14:39 +0000, Moessbauer, Felix (FT RPD CED OES-
DE) wrote:
> On Tue, 2025-10-07 at 12:39 +0200, Florian Bezdeka wrote:
> > On Mon, 2025-10-06 at 16:12 +0200, 'Felix Moessbauer' via isar-users
> > wrote:
> > > The ISAR_APT_SNAPSHOT_DATE variable controls the timestamp of the
> > > sources on the snapshot mirror. This variable further is a shortcut for
> > > ISAR_APT_SNAPSHOT_TIMESTAMP that avoids letting the user compute a unix
> > > timestamp of the desired date. However, while
> > > ISAR_APT_SNAPSHOT_TIMESTAMP is always available via bitbake conf, the
> > > _DATE variant is only available after computing it in do_bootstrap.
> > > 
> > > In 394e954, support was added to just set the timestamp / date for the
> > > security distribution by extending the _TIMESTAMP and _DATE variables
> >   ^^^^^^^^^^^^^^^^^^^^
> > Wording: distribution -> repository?
> > 
> > According to https://wiki.debian.org/SourcesList this seems the right
> > term here.
> 
> But there they also state:
> 
> Apt downloads and installs packages from one or more software
> repositories (sometimes called suites, distributions, versions, or even
> sources). 

Examples:

Distribution: Debian
Suites: trixie trixie-updates
Components: main non-free-firmware

Repository: The package feed for one specific combination of the
definitions above.

Wording is important here as this bug might trigger security
implications. We might not have build what the user expected. So we
have to clearly communicate what the problem was - by using the right
wording.

> 
> As we currently call this dist / distribution in the source code, I
> prefer to also use that term in the commit message.
> 
> If a debian policy states the name, I would change it, but just because
> it is mentioned in the wiki it not enough to require the change.
> 
> Felix
> 
> > 
> > > with a flag. However, the logic that evaluates the variable in
> > > DISTRO_APT_SNAPSHOT_PREMIRROR is defined in the distro conf, which is
> > > parsed earlier than the logic in do_bootstrap. By that, depending on the
> > > context the variable might not be set leading to incorrect substitutions
> > > of the apt-sources. This has not been observed in actual images, but in
> > > the devshell and when printing the variable with bitbake.
> > > 
> > > Anyways, the value of the variable should not depend on the parsing
> > > order. To fix this, we now do not expand the variable until using it.
> > 
> > Agree.
> > 
> > > 
> > > Fixes: 394e9540 ("snapshots: add option to use separate timestamp ...")
> > > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > > ---
> > >  meta/conf/distro/debian-common.conf | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/meta/conf/distro/debian-common.conf b/meta/conf/distro/debian-common.conf
> > > index 8da17d6f..92daddc3 100644
> > > --- a/meta/conf/distro/debian-common.conf
> > > +++ b/meta/conf/distro/debian-common.conf
> > > @@ -45,6 +45,6 @@ DISTRO_BOOTSTRAP_BASE_PACKAGES:append:bullseye = " usrmerge"
> > >  
> > >  # snapshot mirror for reproducible builds
> > >  DISTRO_APT_SNAPSHOT_PREMIRROR ??= " \
> > > -    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security')}\n \
> > > +    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security', expand=False)}\n \
> > >      deb.debian.org/(.*)/? snapshot.debian.org/archive/\1/${ISAR_APT_SNAPSHOT_DATE}\n \
> > >  "
> > > -- 
> > > 2.51.0
> > > 
> > > -- 
> > > You received this message because you are subscribed to the Google Groups "isar-users" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
> > > To view this discussion visit https://groups.google.com/d/msgid/isar-users/20251006141227.1017941-1-felix.moessbauer%40siemens.com.
> 
> -- 
> Siemens AG
> Linux Expert Center
> Friedrich-Ludwig-Bauer-Str. 3
> 85748 Garching, Germany
MOESSBAUER, Felix Oct. 7, 2025, 3:20 p.m. UTC | #4
On Tue, 2025-10-07 at 16:57 +0200, Florian Bezdeka wrote:
> On Tue, 2025-10-07 at 14:39 +0000, Moessbauer, Felix (FT RPD CED OES-
> DE) wrote:
> > On Tue, 2025-10-07 at 12:39 +0200, Florian Bezdeka wrote:
> > > On Mon, 2025-10-06 at 16:12 +0200, 'Felix Moessbauer' via isar-users
> > > wrote:
> > > > The ISAR_APT_SNAPSHOT_DATE variable controls the timestamp of the
> > > > sources on the snapshot mirror. This variable further is a shortcut for
> > > > ISAR_APT_SNAPSHOT_TIMESTAMP that avoids letting the user compute a unix
> > > > timestamp of the desired date. However, while
> > > > ISAR_APT_SNAPSHOT_TIMESTAMP is always available via bitbake conf, the
> > > > _DATE variant is only available after computing it in do_bootstrap.
> > > > 
> > > > In 394e954, support was added to just set the timestamp / date for the
> > > > security distribution by extending the _TIMESTAMP and _DATE variables
> > >   ^^^^^^^^^^^^^^^^^^^^
> > > Wording: distribution -> repository?
> > > 
> > > According to https://wiki.debian.org/SourcesList this seems the right
> > > term here.
> > 
> > But there they also state:
> > 
> > Apt downloads and installs packages from one or more software
> > repositories (sometimes called suites, distributions, versions, or even
> > sources). 
> 
> Examples:
> 
> Distribution: Debian
> Suites: trixie trixie-updates
> Components: main non-free-firmware
> 
> Repository: The package feed for one specific combination of the
> definitions above.
> 
> Wording is important here as this bug might trigger security
> implications. We might not have build what the user expected. So we
> have to clearly communicate what the problem was - by using the right
> wording.

I leave it up to the maintainers to change the commit message.
Again, if you can cite the location in the debian policy where this is
stated I will immediately change it. But if that is not specified,
there is no "right wording".

Felix

> 
> > 
> > As we currently call this dist / distribution in the source code, I
> > prefer to also use that term in the commit message.
> > 
> > If a debian policy states the name, I would change it, but just because
> > it is mentioned in the wiki it not enough to require the change.
> > 
> > Felix
> > 
> > > 
> > > > with a flag. However, the logic that evaluates the variable in
> > > > DISTRO_APT_SNAPSHOT_PREMIRROR is defined in the distro conf, which is
> > > > parsed earlier than the logic in do_bootstrap. By that, depending on the
> > > > context the variable might not be set leading to incorrect substitutions
> > > > of the apt-sources. This has not been observed in actual images, but in
> > > > the devshell and when printing the variable with bitbake.
> > > > 
> > > > Anyways, the value of the variable should not depend on the parsing
> > > > order. To fix this, we now do not expand the variable until using it.
> > > 
> > > Agree.
> > > 
> > > > 
> > > > Fixes: 394e9540 ("snapshots: add option to use separate timestamp ...")
> > > > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > > > ---
> > > >  meta/conf/distro/debian-common.conf | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/meta/conf/distro/debian-common.conf b/meta/conf/distro/debian-common.conf
> > > > index 8da17d6f..92daddc3 100644
> > > > --- a/meta/conf/distro/debian-common.conf
> > > > +++ b/meta/conf/distro/debian-common.conf
> > > > @@ -45,6 +45,6 @@ DISTRO_BOOTSTRAP_BASE_PACKAGES:append:bullseye = " usrmerge"
> > > >  
> > > >  # snapshot mirror for reproducible builds
> > > >  DISTRO_APT_SNAPSHOT_PREMIRROR ??= " \
> > > > -    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security')}\n \
> > > > +    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security', expand=False)}\n \
> > > >      deb.debian.org/(.*)/? snapshot.debian.org/archive/\1/${ISAR_APT_SNAPSHOT_DATE}\n \
> > > >  "
> > > > -- 
> > > > 2.51.0
> > > > 
> > > > -- 
> > > > You received this message because you are subscribed to the Google Groups "isar-users" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
> > > > To view this discussion visit https://groups.google.com/d/msgid/isar-users/20251006141227.1017941-1-felix.moessbauer%40siemens.com.
> > 
> > -- 
> > Siemens AG
> > Linux Expert Center
> > Friedrich-Ludwig-Bauer-Str. 3
> > 85748 Garching, Germany
Zhihang Wei Oct. 10, 2025, 4 p.m. UTC | #5
Both patches were applied to next, thanks.

On 10/6/25 16:12, 'Felix Moessbauer' via isar-users wrote:
> The ISAR_APT_SNAPSHOT_DATE variable controls the timestamp of the
> sources on the snapshot mirror. This variable further is a shortcut for
> ISAR_APT_SNAPSHOT_TIMESTAMP that avoids letting the user compute a unix
> timestamp of the desired date. However, while
> ISAR_APT_SNAPSHOT_TIMESTAMP is always available via bitbake conf, the
> _DATE variant is only available after computing it in do_bootstrap.
>
> In 394e954, support was added to just set the timestamp / date for the
> security distribution by extending the _TIMESTAMP and _DATE variables
> with a flag. However, the logic that evaluates the variable in
> DISTRO_APT_SNAPSHOT_PREMIRROR is defined in the distro conf, which is
> parsed earlier than the logic in do_bootstrap. By that, depending on the
> context the variable might not be set leading to incorrect substitutions
> of the apt-sources. This has not been observed in actual images, but in
> the devshell and when printing the variable with bitbake.
>
> Anyways, the value of the variable should not depend on the parsing
> order. To fix this, we now do not expand the variable until using it.
>
> Fixes: 394e9540 ("snapshots: add option to use separate timestamp ...")
> Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> ---
>   meta/conf/distro/debian-common.conf | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/conf/distro/debian-common.conf b/meta/conf/distro/debian-common.conf
> index 8da17d6f..92daddc3 100644
> --- a/meta/conf/distro/debian-common.conf
> +++ b/meta/conf/distro/debian-common.conf
> @@ -45,6 +45,6 @@ DISTRO_BOOTSTRAP_BASE_PACKAGES:append:bullseye = " usrmerge"
>   
>   # snapshot mirror for reproducible builds
>   DISTRO_APT_SNAPSHOT_PREMIRROR ??= " \
> -    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security')}\n \
> +    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security', expand=False)}\n \
>       deb.debian.org/(.*)/? snapshot.debian.org/archive/\1/${ISAR_APT_SNAPSHOT_DATE}\n \
>   "

Patch

diff --git a/meta/conf/distro/debian-common.conf b/meta/conf/distro/debian-common.conf
index 8da17d6f..92daddc3 100644
--- a/meta/conf/distro/debian-common.conf
+++ b/meta/conf/distro/debian-common.conf
@@ -45,6 +45,6 @@  DISTRO_BOOTSTRAP_BASE_PACKAGES:append:bullseye = " usrmerge"
 
 # snapshot mirror for reproducible builds
 DISTRO_APT_SNAPSHOT_PREMIRROR ??= " \
-    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security')}\n \
+    deb.debian.org/(debian-security)/? snapshot.debian.org/archive/\1/${@d.getVarFlag('ISAR_APT_SNAPSHOT_DATE', 'security', expand=False)}\n \
     deb.debian.org/(.*)/? snapshot.debian.org/archive/\1/${ISAR_APT_SNAPSHOT_DATE}\n \
 "