Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[master] deb: cleanup makefile, remove unused GITCOMMIT, and pass VERSION_ID through Dockerfile #817

Merged
merged 3 commits into from
Jan 11, 2023

Conversation

thaJeztah
Copy link
Member

deb: remove unused GITCOMMIT make var

It doesn't appear to be used anywhere

deb: Makefile: sort vars similar to rpm variant

For easier comparing :)

deb: pass VERSION_ID through Dockerfile

We'll be using VERSION_ID in other places, so adding it in the Dockerfile makes
sure it's always present, without having to depend on /etc/os-release.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For easier comparing :)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
We'll be using VERSION_ID in other places, so adding it in the Dockerfile makes
sure it's always present, without having to depend on /etc/os-release.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

@crazy-max @vvoland PTAL 🤗

@crazy-max
Copy link
Member

crazy-max commented Jan 11, 2023

I was looking at docker/packaging, and it seems I don't set the version ID atm: https://github.com/docker/packaging/blob/0670638bca56b14897acb897deea11b8a9e46787/pkg/containerd/scripts/pkg-deb-build.sh#L61

└── [4.0K]  ubuntu
    ├── [4.0K]  bionic
    │   ├── [4.0K]  amd64
    │   │   ├── [2.7M]  containerd.io-dbgsym_1.6.14-1_amd64.ddeb
    │   │   ├── [5.6K]  containerd.io_1.6.14-1_amd64.buildinfo
    │   │   ├── [1.3K]  containerd.io_1.6.14-1_amd64.changes
    │   │   └── [ 26M]  containerd.io_1.6.14-1_amd64.deb
    │   ├── [4.0K]  arm
    │   │   └── [4.0K]  v7
    │   │       ├── [2.6M]  containerd.io-dbgsym_1.6.14-1_armhf.ddeb
    │   │       ├── [5.4K]  containerd.io_1.6.14-1_armhf.buildinfo
    │   │       ├── [1.3K]  containerd.io_1.6.14-1_armhf.changes
    │   │       └── [ 19M]  containerd.io_1.6.14-1_armhf.deb
    │   ├── [4.0K]  arm64
    │   │   ├── [2.6M]  containerd.io-dbgsym_1.6.14-1_arm64.ddeb
    │   │   ├── [5.5K]  containerd.io_1.6.14-1_arm64.buildinfo
    │   │   ├── [1.3K]  containerd.io_1.6.14-1_arm64.changes
    │   │   └── [ 19M]  containerd.io_1.6.14-1_arm64.deb
    │   └── [4.0K]  s390x
    │       ├── [2.7M]  containerd.io-dbgsym_1.6.14-1_s390x.ddeb
    │       ├── [5.4K]  containerd.io_1.6.14-1_s390x.buildinfo
    │       ├── [1.3K]  containerd.io_1.6.14-1_s390x.changes
    │       └── [ 20M]  containerd.io_1.6.14-1_s390x.deb

@thaJeztah
Copy link
Member Author

Yes, for containerd it's missing (see docker/containerd-packaging#303), and we should add it there as well. Not sure if we can do it without "penalty" for the current (1.6.x) version (maybe we can). For the plugins we should also fix these (I'm working on some of that right now).

@@ -1,6 +1,7 @@
ARG GO_IMAGE
ARG DISTRO=ubuntu
ARG SUITE=bionic
ARG VERSION_ID=18.04
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it should be 1804 instead?

Copy link
Member

Choose a reason for hiding this comment

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

Ah nevermind that's the right ID

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! Missed your comment; it looks like deb / dpkg is happy with the current (CalVer) format;

dpkg --compare-versions "23.0.0~rc.1-0~ubuntu.22.10.0~kinetic" ">>" "23.0.0~rc.1-0~ubuntu.22.04.0~jammy" && echo "OK" || echo "KO"
OK

dpkg --compare-versions "22.10.0~kinetic" ">>" "22.04.0~jammy" && echo "OK" || echo "KO"
OK

dpkg --compare-versions "22.10" ">>" "22.04" && echo "OK" || echo "KO"
OK

@thaJeztah
Copy link
Member Author

Let me bring this one in; more to follow, but wanted to get the "preparation" steps already merged so that we can focus on the actual fixes 👍

@thaJeztah thaJeztah merged commit f0a887d into docker:master Jan 11, 2023
@thaJeztah thaJeztah deleted the deb_changes branch January 11, 2023 12:38
@thaJeztah thaJeztah changed the title deb: cleanup makefile, remove unused GITCOMMIT, and pass VERSION_ID through Dockerfile [master] deb: cleanup makefile, remove unused GITCOMMIT, and pass VERSION_ID through Dockerfile Jan 11, 2023
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.

2 participants