docker-library / buildpack-deps

MIT License
445 stars 113 forks source link

Add Ubuntu Hirsute #117

Closed vicamo closed 3 years ago

vicamo commented 3 years ago

Signed-off-by: You-Sheng Yang vicamo@gmail.com

tianon commented 3 years ago

Doh, looks like the point at which tzdata gets installed has moved, so we also need something like this:

diff --git a/Dockerfile-scm.template b/Dockerfile-scm.template
index f40be20..56afc40 100644
--- a/Dockerfile-scm.template
+++ b/Dockerfile-scm.template
@@ -1,7 +1,13 @@
 FROM buildpack-deps:{{ env.codename }}-curl

 # procps is very common in build systems, and is a reasonably small package
-RUN apt-get update && apt-get install -y --no-install-recommends \
+RUN set -eux; \
+   apt-get update; \
+{{ if "ubuntu" == env.dist and (["bionic", "focal", "groovy", "xenial"] | index(env.codename) | not) then ( -}}
+# make sure debconf doesn't try to prompt (e.g. tzdata on Ubuntu)
+   DEBIAN_FRONTEND=noninteractive \
+{{ ) else "" end -}}
+   apt-get install -y --no-install-recommends \
 {{
 if [
    "jessie", "stretch",
@@ -16,4 +22,5 @@ if [
        subversion \
        \
        procps \
-   && rm -rf /var/lib/apt/lists/*
+   ; \
+   rm -rf /var/lib/apt/lists/*
diff --git a/Dockerfile.template b/Dockerfile.template
index 9bbe9cf..d876b09 100644
--- a/Dockerfile.template
+++ b/Dockerfile.template
@@ -2,7 +2,7 @@ FROM buildpack-deps:{{ env.codename }}-scm

 RUN set -ex; \
    apt-get update; \
-{{ if "ubuntu" == env.dist then ( -}}
+{{ if "ubuntu" == env.dist and (["bionic", "focal", "groovy", "xenial"] | index(env.codename)) then ( -}}
 # make sure debconf doesn't try to prompt (e.g. tzdata on Ubuntu)
    DEBIAN_FRONTEND=noninteractive \
 {{ ) else "" end -}}

(Happy to take over and add this/apply templates if you'd prefer; thanks for the contribution! :+1:)

vicamo commented 3 years ago

Doh, looks like the point at which tzdata gets installed has moved, so we also need something like this:

libpython3.9-stdlib depends on tzdata now, which follows Debian Sid/Testing would probably have the same issue soon. https://packages.debian.org/sid/libpython3.9-stdlib So what about running apt-get with DEBIAN_FRONTEND=noninteractive always without those if-statements?

vicamo commented 3 years ago

Added a commit that always run apt-get install with DEBIAN_FRONTEND=noninteractive.

tianon commented 3 years ago

Doh, looks like the point at which tzdata gets installed has moved, so we also need something like this:

libpython3.9-stdlib depends on tzdata now, which follows Debian Sid/Testing would probably have the same issue soon. https://packages.debian.org/sid/libpython3.9-stdlib So what about running apt-get with DEBIAN_FRONTEND=noninteractive always without those if-statements?

Hmm, if that's the case, shouldn't https://github.com/docker-library/buildpack-deps/runs/1519544260 have failed?

tianon commented 3 years ago

The other way we could take this is to just install tzdata in the curl variants (which IMO makes sense) and drop DEBIAN_FRONTEND entirely.

tianon commented 3 years ago

(Which should then get it installed early enough to avoid https://bugs.debian.org/929417)

tianon commented 3 years ago

That also points out why we don't see issues with this in Debian:

tzdata is already the newest version (2020d-1).
tianon commented 3 years ago

Can confirm the following diff works fine for Hirsute though:

diff --git a/Dockerfile-curl.template b/Dockerfile-curl.template
index dc93630..8b014e5 100644
--- a/Dockerfile-curl.template
+++ b/Dockerfile-curl.template
@@ -1,11 +1,15 @@
 FROM {{ env.dist }}:{{ env.codename }}

-RUN apt-get update && apt-get install -y --no-install-recommends \
+RUN set -eux; \
+   apt-get update; \
+   apt-get install -y --no-install-recommends \
        ca-certificates \
        curl \
        netbase \
+       tzdata \
        wget \
-   && rm -rf /var/lib/apt/lists/*
+   ; \
+   rm -rf /var/lib/apt/lists/*

 RUN set -ex; \
    if ! command -v gpg > /dev/null; then \
tianon commented 3 years ago

(and works and/or is harmless elsewhere)

vicamo commented 3 years ago

@tianon is there any reason not to use DEBIAN_FRONTEND in Dockerfile? I think that's pretty correct as there just won't have any interaction in Docker build, so specifying DEBIAN_FRONTEND=noninteractive here seems a right solution to me in comparison to all other hacks just for one specific case. tzdata is not something supposed to be installed here explicitly. It's a depending package brought in through others indirectly. There could be more similar cases, so I'd suggest a general solution instead.

In summary, I'd suggest:

  1. Docker build is an non-interactive process, so export DEBIAN_FRONTEND=noninteractive when executing apt-get.
  2. Don't export DEBIAN_FRONTEND as a docker image env as docker containers may be run interactively,

Doh, looks like the point at which tzdata gets installed has moved, so we also need something like this:

libpython3.9-stdlib depends on tzdata now, which follows Debian Sid/Testing would probably have the same issue soon. https://packages.debian.org/sid/libpython3.9-stdlib So what about running apt-get with DEBIAN_FRONTEND=noninteractive always without those if-statements?

Hmm, if that's the case, shouldn't https://github.com/docker-library/buildpack-deps/runs/1519544260 have failed?

tzdata has been installed in sid and testing base image. So, yes, my previous claim that this might happen to Sid/Testing as well is not so correct.

tianon commented 3 years ago

Yeah, DEBIAN_FRONTEND=noninteractive has always bothered me because debconf should really be detecting it properly -- there's no stdin, so it's already implied noninteractive (https://bugs.debian.org/929417).

It's a bit pedantic, but I'd rather have a slightly more verbose template (I'd be totally in favor of only adding tzdata explicitly on Ubuntu). :see_no_evil:

vicamo commented 3 years ago

Yeah, DEBIAN_FRONTEND=noninteractive has always bothered me because debconf should really be detecting it properly -- there's no stdin, so it's already implied noninteractive (https://bugs.debian.org/929417).

It's a bit pedantic, but I'd rather have a slightly more verbose template (I'd be totally in favor of only adding tzdata explicitly on Ubuntu).

Would you land the fix first so I can rebase on it or maybe take over this completely?