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] rpm: drop unnecessary build-deps #1010

Merged
merged 2 commits into from
Dec 26, 2024

Conversation

neersighted
Copy link
Member

@neersighted neersighted commented Apr 2, 2024

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah
Copy link
Member

Also curious if we need to look at updating the Standards-Version, and possibly if there's improvements we can use from newer versions; https://www.debian.org/doc/debian-policy/upgrading-checklist.html#version-3-9-6

At least I spotted some "-Arch" options (perhaps useful if we need architecture specific handling), and saw some mentions of fallbacks that are no longer needed.

Standards-Version: 3.9.6

@thaJeztah
Copy link
Member

Did a quick rebase after #1124 was merged

@thaJeztah thaJeztah marked this pull request as ready for review December 26, 2024 12:06
BuildRequires: libtool
BuildRequires: libtool-ltdl-devel
BuildRequires: make
BuildRequires: pkgconfig
BuildRequires: pkgconfig(systemd)
BuildRequires: systemd-devel
BuildRequires: tar
BuildRequires: which
Copy link
Member

Choose a reason for hiding this comment

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

This one was originally added in fda74c4 (through #314).

My guess is that some script at some point incorrectly used which <some command> instead of command -v <some command>, but I couldn't find a reference to it either at the commit that added it, nor at current master, so I'm gonna assume here that it was never needed at all;

git rev-parse --verify HEAD
fda74c4df83554ae2662184b4a5a1a1eaaf6927a

git grep 'which '
deb/common/docker-ce.docker.init:       # see also init_is_upstart in /lib/lsb/init-functions (which isn't available in Ubuntu 12.04, or we'd use it directly)
deb/gen-deb-ver:    # be compared to determine which happened later, the commit hash identifes
rpm/gen-rpm-ver:    # be compared to determine which happened later, the commit hash identifes
static/gen-static-ver:    # be compared to determine which happened later, the commit hash identifes

git grep 'command '
deb/common/docker-ce.postinst:          echo "You can use the 'docker engine update' command to update your system, or switch to using the Enterprise packages."
deb/common/rules:       dh $@ --with=bash-completion $(shell command -v dh_systemd_enable > /dev/null 2>&1 && echo --with=systemd)
deb/gen-deb-ver:    # as a pre-release before version v0.0.0, so that the go command prefers any
rpm/gen-rpm-ver:    # as a pre-release before version v0.0.0, so that the go command prefers any
static/gen-static-ver:    # as a pre-release before version v0.0.0, so that the go command prefers any


git checkout master

git rev-parse --verify HEAD
2f0c865a12578226abe568ca772c9ca06e58516b

git grep 'which '
Jenkinsfile:        // Otherwise it accidentally pulls armel images which then breaks the verify step
Jenkinsfile:                // in Jenkins' BlueOcean view, which truncates names....
Jenkinsfile:                // in Jenkins' BlueOcean view, which truncates names....
Jenkinsfile:                // in Jenkins' BlueOcean view, which truncates names....
Jenkinsfile:                // in Jenkins' BlueOcean view, which truncates names....
LICENSE:      form, that is based on (or derived from) the Work and for which the
LICENSE:      with the Work to which such Contribution(s) was submitted. If You
README.md:which can be found in the "rpm" and "deb" subdirectories, as well as scripts to
deb/build-deb:# - pkgRevision (usually "-0", see above), which allows updating packages with
deb/build-deb:#   we prefix the codename with a tilde (~), which effectively excludes it from
deb/build-deb:# EPOCH because of our use of CalVer, which made version comparing not work in
deb/common/rules:# zstd compression, which is non-standard, and breaks 'dpkg-sig --verify'
deb/gen-deb-ver:# non-pre-release versions, which would prevent users from updating from a pre-
deb/gen-deb-ver:        # be compared to determine which happened later, the commit hash identifes
deb/gen-deb-ver:                # Using BSD date (macOS), which doesn't support the --date option
rpm/SPECS/docker-ce.spec:# Provides modprobe, which we use to load br_netfilter if not loaded.
rpm/gen-rpm-ver:# - Version: > 1 : Only to be used for packaging-only changes (new package built for a version for which a package was already built/released)
rpm/gen-rpm-ver:# in this field, which is still allowed, but considered deprecated; see
rpm/gen-rpm-ver:# non-pre-release versions, which would prevent users from updating from a pre-
rpm/gen-rpm-ver:# > The tilde symbol (‘~’) is used before a version component which must sort
rpm/gen-rpm-ver:# > which wouldn’t otherwise sort appropriately.
rpm/gen-rpm-ver:        # be compared to determine which happened later, the commit hash identifes
rpm/gen-rpm-ver:                # Using BSD date (macOS), which doesn't support the --date option
static/gen-static-ver:  # be compared to determine which happened later, the commit hash identifes
static/gen-static-ver:          # Using BSD date (macOS), which doesn't support the --date option
verify:                         # may still be using "rawhide", which is not present on our package

git grep 'command '
deb/gen-deb-ver:        # as a pre-release before version v0.0.0, so that the go command prefers any
install-containerd-helpers:     if command -v dnf5; then
install-containerd-helpers:     elif command -v dnf; then
rpm/gen-rpm-ver:        # as a pre-release before version v0.0.0, so that the go command prefers any
static/gen-static-ver:  # as a pre-release before version v0.0.0, so that the go command prefers any

@@ -34,15 +34,13 @@ BuildRequires: cmake
BuildRequires: gcc
BuildRequires: git
BuildRequires: glibc-static
BuildRequires: libarchive
Copy link
Member

Choose a reason for hiding this comment

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

This one was added in 7824214, as part of #554

It was to fix a failure when compiling tini;

According to https://bugs.centos.org/view.php?id=18212, upgrading to libarchive-3.3.3-1.el8.x86_64 should resolve the problem.

Looks indeed that it's no longer needed (or at least, CI is happy), but perhaps I should make this a separate commit that's a clean revert of the other one to help correlate / track down history.

thaJeztah and others added 2 commits December 26, 2024 13:30
…r_zstd)

This reverts commit 7824214.

That change was  added to fix a missing archive_write_add_filter_zstd  when
compiling tini, but it's no longer needed;

> Add libarchive build-dep to fix missing archive_write_add_filter_zstd
>
> Trying to fix
>
>     + echo 'Install tini version de40ad007797e0dcd8b7126f27bb87401d224240'
>     + git clone https://github.com/krallin/tini.git /go/tini
>     Install tini version de40ad007797e0dcd8b7126f27bb87401d224240
>     Cloning into '/go/tini'...
>     + cd /go/tini
>     + git checkout -q de40ad007797e0dcd8b7126f27bb87401d224240
>     + cmake .
>     cmake: symbol lookup error: cmake: undefined symbol: archive_write_add_filter_zstd
>     error: Bad exit status from /var/tmp/rpm-tmp.Dl5CDf (%build)
>
> According to https://bugs.centos.org/view.php?id=18212, upgrading to libarchive-3.3.3-1.el8.x86_64
> should resolve the problem.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah changed the title rpm,deb: drop unnecessary build-deps rpm: drop unnecessary build-deps Dec 26, 2024
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

let's bring this in

@thaJeztah thaJeztah merged commit e47d637 into docker:master Dec 26, 2024
8 checks passed
@thaJeztah thaJeztah changed the title rpm: drop unnecessary build-deps [master] rpm: drop unnecessary build-deps Dec 26, 2024
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