Skip to content

deb: keep '/var/lib/apt/lists/' to allow building for Debian unstable#219

Merged
tiborvass merged 1 commit into
docker:masterfrom
thaJeztah:fix_unstable
Feb 22, 2021
Merged

deb: keep '/var/lib/apt/lists/' to allow building for Debian unstable#219
tiborvass merged 1 commit into
docker:masterfrom
thaJeztah:fix_unstable

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

relates to #213 (comment)

Debian "unstable" releases use apt caching information to get the codename
see discussion on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=845651:

That's all to say that this bug is (to my belief) actually expected behaviour;
and fixing it through forcing the codename to be interpreted as "stretch" when
apt-cache information is unavailable would be wrong. When /etc/debian_version
contains "potato/sid", the codename is either potato xor sid, and only apt-
cache can discriminate a testing host from a sid host. Therefore, in such a
situation, the correct answer is actually "I can't tell", aka "n/a".

From testing, it reads the information from these files:

  • /var/lib/apt/lists/deb.debian.org_debian_dists_bullseye_InRelease
  • /var/lib/apt/lists/deb.debian.org_debian_dists_bullseye_main_binary-amd64_Packages.lz4

Removing these files (rm -rf /var/lib/apt/lists/*) causes 'lsb_release -sc`
to print 'n/a'. While we could use '/etc/debian_version' as a fallback for our
own scripts (stripping everything after '/' (e.g. bullseye/sid -> bullseye),
dpkg-buildpackage will still depend on this information to be present, and
if not present, renames packages to use 'n/a' in their path:

dpkg-buildpackage: info: full upload; Debian-native package (full source is included)
renamed '../containerd.io-dbgsym_0.20210219.014044~e58be59-1_amd64.deb' -> '/build/debian/n/a/amd64/containerd.io-dbgsym_0.20210219.014044~e58be59-1_amd64.deb'
renamed '../containerd.io_0.20210219.014044~e58be59-1_amd64.deb' -> '/build/debian/n/a/amd64/containerd.io_0.20210219.014044~e58be59-1_amd64.deb'

Given that we don't need the final image (as we only use it as a build environment
and copy the artifacts out), keeping some of the cache files should not be a problem.

# and copy the artifacts out), keeping some of the cache files should not be a problem.
RUN apt-get update -q \
&& mk-build-deps -t "apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y" -i debian/control \
&& apt-get clean \
Copy link
Copy Markdown
Contributor

@tianon tianon Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&& mk-build-deps -t "apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y" -i debian/control \
&& apt-get clean \
&& rm -rf /var/cache/apt /var/lib/apt/lists/*
&& rm -rf /var/cache/apt
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... which means this is a no-op too (and redundant with apt-get clean in a normal install anyhow)

tianon
tianon previously approved these changes Feb 19, 2021
Copy link
Copy Markdown
Contributor

@tianon tianon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess #220 (review) means I give this a transitive LGTM 😅

Comment thread scripts/build-deb Outdated
@thaJeztah
Copy link
Copy Markdown
Member Author

I'll take your LGTM (CI is 💚)

Debian "unstable" releases use apt caching information to get the codename
see discussion on https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=845651:

> That's all to say that this bug is (to my belief) actually expected behaviour;
> and fixing it through forcing the codename to be interpreted as "stretch" when
> apt-cache information is unavailable would be wrong. When /etc/debian_version
> contains "potato/sid", the codename is either potato xor sid, and only apt-
> cache can discriminate a testing host from a sid host. Therefore, in such a
> situation, the correct answer is actually "I can't tell", aka "n/a".

From testing, it reads the information from these files:

  - /var/lib/apt/lists/deb.debian.org_debian_dists_bullseye_InRelease
  - /var/lib/apt/lists/deb.debian.org_debian_dists_bullseye_main_binary-amd64_Packages.lz4

Removing these files (`rm -rf /var/lib/apt/lists/*`) causes 'lsb_release -sc`
to print 'n/a'. While we could use '/etc/debian_version' as a fallback for our
own scripts (stripping everything after '/' (e.g. bullseye/sid -> bullseye),
dpkg-buildpackage will still depend on this information to be present, and
if not present, renames packages to use 'n/a' in their path:

    dpkg-buildpackage: info: full upload; Debian-native package (full source is included)
    renamed '../containerd.io-dbgsym_0.20210219.014044~e58be59-1_amd64.deb' -> '/build/debian/n/a/amd64/containerd.io-dbgsym_0.20210219.014044~e58be59-1_amd64.deb'
    renamed '../containerd.io_0.20210219.014044~e58be59-1_amd64.deb' -> '/build/debian/n/a/amd64/containerd.io_0.20210219.014044~e58be59-1_amd64.deb'

Given that we don't need the final image (as we only use it as a build environment
and copy the artifacts out), keeping some of the cache files should not be a problem.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Copy link
Copy Markdown
Contributor

@tianon tianon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-LGTM 👍

@tiborvass tiborvass merged commit e0d5cbb into docker:master Feb 22, 2021
@thaJeztah thaJeztah deleted the fix_unstable branch February 22, 2021 18:39
thaJeztah added a commit to thaJeztah/docker-ce-packaging that referenced this pull request May 19, 2022
Debian "sid" is not an actual distro version. Sid represents the "unstable"
channel ("next stable in-progress"). Currently, it is equivalent to "bullseye",
but once "bullseye" is stable, it becomes "bookworm", "trixy", etc (see the
list of Debian releases at https://wiki.debian.org/DebianReleases).

We should fix this hard-coded override, and instead try to get this name from
information in /var/lib/apt/lists/.

Also see docker/containerd-packaging#219 for details.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants