Add LICENSE file to RPMs #267

matthewtorr-msft · 2023-10-23T14:58:42Z

Fixes #266

This adds the LICENSE files to RPMs.

Unfortunately I haven't been able to get the RPM builds working locally so I've not been able to test it, though it should "just work". (That assumes all target distros have rpm 4.11 or above; if not, %doc would be a suitable alternative.)

nwneisen

Thanks @matthewtorr-msft! I'll let CI run on it first and then check it out to take a closer look.

We should also do this for the other types of installers so that we are consistent throughout the project.

packaging/rpm/SPECS/cri-dockerd.spec

nwneisen · 2023-10-24T14:58:33Z

@matthewtorr-msft After checking out the RPM docs, I agree that your change should be working.

However, when I build locally with make rpm, I get an error that the LICENSE file could not be found.

error: File not found: /root/rpmbuild/BUILDROOT/cri-dockerd-0.3.6.20231020153952.a4152906-0.fc36.x86_64/usr/share/licenses/cri-dockerd/LICENSE

RPM build errors:
    File not found: /root/rpmbuild/BUILDROOT/cri-dockerd-0.3.6.20231020153952.a4152906-0.fc36.x86_64/usr/share/licenses/cri-dockerd/LICENSE
make[2]: *** [Makefile:74: fedora-36] Error 1

Could I have you take a look at it? Also, could you share your error message if the build is not working for you?

Thanks!

matthewtorr-msft · 2023-10-24T15:35:03Z

If I run make rpm from packaging/rpm, it errors out with the following:

docker run --privileged --rm -i -e PLATFORM -v /home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm/rpmbuild/SOURCES:/root/rpmbuild/SOURCES -v /home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm/rpmbuild/RPMS:/root/rpmbuild/RPMS -v /home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm/rpmbuild/SRPMS:/root/rpmbuild/SRPMS rpmbuild-fedora-36/x86_64 -ba --define '_gitcommit 4c3d89e1' --define '_release 0' --define '_version 0.3.6.20231023145356.4c3d89e1' --define '_origversion 0.3.6-20231023145356-4c3d89e1' --define '_buildldflags ' SPECS/cri-dockerd.spec
error: Macro %_buildldflags has empty body
make: *** [Makefile:74: fedora-36] Error 1

If I run from the root of the repo, here is the whole output which has several kinds of errors:

fatal: No names found, cannot describe anything.
fatal: No names found, cannot describe anything.
make APP_DIR=/home/developer/github.com/matthewtorr-msft/cri-dockerd -C /home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging rpm
fatal: No names found, cannot describe anything.
make[1]: Entering directory '/home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging'
make -C rpm APP_DIR=/home/developer/github.com/matthewtorr-msft/cri-dockerd GO_VERSION=1.18.3 rpm
make[2]: Entering directory '/home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm'
+ APP_DIR=/home/developer/github.com/matthewtorr-msft/cri-dockerd
+ VERSION=
+ [[ 1 < 2 ]]
+ echo 'not enough args'
+ exit 1
+ APP_DIR=/home/developer/github.com/matthewtorr-msft/cri-dockerd
+ VERSION=
+ [[ 1 < 2 ]]
+ echo 'not enough args'
+ exit 1
+ APP_DIR=/home/developer/github.com/matthewtorr-msft/cri-dockerd
+ VERSION=
+ [[ 1 < 2 ]]
+ echo 'not enough args'
+ exit 1
+ APP_DIR=/home/developer/github.com/matthewtorr-msft/cri-dockerd
+ VERSION=
+ [[ 1 < 2 ]]
+ echo 'not enough args'
+ exit 1
+ APP_DIR=/home/developer/github.com/matthewtorr-msft/cri-dockerd
+ VERSION=
+ [[ 1 < 2 ]]
+ echo 'not enough args'
+ exit 1
/home/developer/github.com/matthewtorr-msft/cri-dockerd

not enough args
== Building packages for fedora-36 ==
docker run --rm -i -v /home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm:/v -w /v alpine chown -R root:root rpmbuild
DOCKER_BUILDKIT=1 docker build  --build-arg GO_IMAGE=golang:1.18.3-buster -t rpmbuild-fedora-36/x86_64 -f fedora-36/Dockerfile .
[+] Building 0.2s (13/13) FINISHED
 => [internal] load build definition from Dockerfile                                             0.0s
 => => transferring dockerfile: 636B                                                             0.0s
 => [internal] load .dockerignore                                                                0.0s
 => => transferring context: 2B                                                                  0.0s
 => [internal] load metadata for docker.io/library/fedora:36                                     0.1s
 => [internal] load metadata for docker.io/library/golang:1.18.3-buster                          0.1s
 => [stage-1 1/6] FROM docker.io/library/fedora:36@sha256:64cd00a0e2b92d527c0a0954162a73e85f160  0.0s
 => [internal] load build context                                                                0.0s
 => => transferring context: 69B                                                                 0.0s
 => [golang 1/1] FROM docker.io/library/golang:1.18.3-buster@sha256:7acdbec4276ce3c8915ffa03148  0.0s
 => CACHED [stage-1 2/6] RUN dnf install -y rpm-build rpmlint dnf-plugins-core                   0.0s
 => CACHED [stage-1 3/6] COPY SPECS /root/rpmbuild/SPECS                                         0.0s
 => CACHED [stage-1 4/6] RUN dnf builddep -y /root/rpmbuild/SPECS/*.spec                         0.0s
 => CACHED [stage-1 5/6] COPY --from=golang /usr/local/go /usr/local/go                          0.0s
 => CACHED [stage-1 6/6] WORKDIR /root/rpmbuild                                                  0.0s
 => exporting to image                                                                           0.0s
 => => exporting layers                                                                          0.0s
 => => writing image sha256:51cd2672278c118e45473ec84a87f26e2be7694f84639f5de71b0c4b0b7b451f     0.0s
 => => naming to docker.io/rpmbuild-fedora-36/x86_64:latest                                      0.0s
docker run --privileged --rm -i -e PLATFORM -v /home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm/rpmbuild/SOURCES:/root/rpmbuild/SOURCES -v /home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm/rpmbuild/RPMS:/root/rpmbuild/RPMS -v /home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm/rpmbuild/SRPMS:/root/rpmbuild/SRPMS rpmbuild-fedora-36/x86_64 -ba --define '_gitcommit args' --define '_release enough' --define '_version not' --define '_origversion ' --define '_buildldflags  -s -w -buildid=4c3d89e1 -X github.com/Mirantis/cri-dockerd/cmd/version.Version= -X github.com/Mirantis/cri-dockerd/cmd/version.PreRelease= -X github.com/Mirantis/cri-dockerd/cmd/version.GitCommit=4c3d89e1' SPECS/cri-dockerd.spec
error: Macro %_origversion has empty body
make[2]: *** [Makefile:74: fedora-36] Error 1
make[2]: Leaving directory '/home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging/rpm'
make[1]: *** [Makefile:20: rpm] Error 2
make[1]: Leaving directory '/home/developer/github.com/matthewtorr-msft/cri-dockerd/packaging'
make: *** [Makefile:37: rpm] Error 2

afbjorklund · 2023-10-24T15:38:31Z

I think the macros probably only work if you use the default build directory (regular %setup)

Used to mark and/or install files as documentation. Can be used as a
regular attribute on an absolute path, or in "special" form on a path
relative to the build directory which causes the files to be installed
and packaged as documentation. The special form strips all but the last
path component. Thus %doc path/to/docfile installs docfile in the
documentation path.

So you probably need something like app/LICENSE, when not using the regular setup?

%setup -q -c -n src -a 0

Translates to "don't create the usual directory, create something called src and add app to it"
https://rpm-software-management.github.io/rpm/manual/spec.html documents %setup

Not sure why the copy doesn't fail properly, but then again - C and error checking, hmm
rpm-software-management/rpm@55bf9ab

matthewtorr-msft · 2023-10-24T15:44:49Z

I think I understand now how the cri-docker.socket file is packaged in, I'll push some further changes based on that and you can see if it works

afbjorklund · 2023-10-24T15:45:24Z

The other files are using some hack with the "usual paths", instead of the regular SOURCE macros

install -D -m 0644 %{SOURCE1} $RPM_BUILD_ROOT/%{_unitdir}/cri-docker.service
install -D -m 0644 %{SOURCE2} $RPM_BUILD_ROOT/%{_unitdir}/cri-docker.socket

Not sure why they don't use the source files (from packaging/systemd), but another copy of them?

It made sense when we just used a binary tarball (without sources), and not building the go code.

nwneisen · 2023-10-24T15:48:38Z

@matthewtorr-msft I usually run make rpm from the root of the project and it passes the additional information to the build

nwneisen · 2023-10-24T16:19:24Z

I just realized I still had the build fix in place also. Updating the golang version for the updated packages fixes the issue: #268

matthewtorr-msft · 2023-10-25T09:44:51Z

I just realized I still had the build fix in place also. Updating the golang version for the updated packages fixes the issue: #268

Great news, thanks. To be clear, is that with the original patch or the updated version? I'll undo the updates if not needed.

nwneisen · 2023-10-25T16:47:11Z

original patch or the updated version

Without the updated golang version, you will get an error (currently on master) from one of the dependencies for building packages that it needs go 1.19+. None of the LICENSE fill changes have worked for me but bumping that image version will let you test it.

matthewtorr-msft · 2023-10-26T11:44:10Z

I've got this working locally and pushed the changes.

Thanks for the go version tip, I needed that to make it work locally.

However, I also needed to change the VERSION line in packaging/common.mk from "?=" to ":="--some of my errors above were due to VERSION not being defined. I haven't committed this as it seems to be working for you. Unsure why we're seeing different behaviour!

nwneisen

Looks good and built for me locally.

Thanks for the contribution and being patient with getting it merged in!

matthewtorr-msft · 2023-10-26T15:23:27Z

Thanks for the contribution and being patient with getting it merged in!

No worries, thank you for being responsive and helping me get it to work!

afbjorklund · 2023-10-26T16:29:27Z

The hardcoded paths probably doesn't matter as much, since this is only run from the Docker container anyway.

Add LICENSE file to RPMs

4c3d89e

nwneisen requested changes Oct 23, 2023

View reviewed changes

packaging/rpm/SPECS/cri-dockerd.spec Show resolved Hide resolved

Speculative fix for adding LICENSE as a source

0f79cb7

copy LICENSE during build step install of install step

7be16a9

nwneisen approved these changes Oct 26, 2023

View reviewed changes

nwneisen merged commit 05006ab into Mirantis:master Oct 26, 2023
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LICENSE file to RPMs #267

Add LICENSE file to RPMs #267

matthewtorr-msft commented Oct 23, 2023

nwneisen left a comment

nwneisen commented Oct 24, 2023

matthewtorr-msft commented Oct 24, 2023

afbjorklund commented Oct 24, 2023 •

edited

Loading

matthewtorr-msft commented Oct 24, 2023

afbjorklund commented Oct 24, 2023 •

edited

Loading

nwneisen commented Oct 24, 2023 •

edited

Loading

nwneisen commented Oct 24, 2023

matthewtorr-msft commented Oct 25, 2023

nwneisen commented Oct 25, 2023 •

edited

Loading

matthewtorr-msft commented Oct 26, 2023

nwneisen left a comment

matthewtorr-msft commented Oct 26, 2023

afbjorklund commented Oct 26, 2023

Add LICENSE file to RPMs #267

Add LICENSE file to RPMs #267

Conversation

matthewtorr-msft commented Oct 23, 2023

nwneisen left a comment

Choose a reason for hiding this comment

nwneisen commented Oct 24, 2023

matthewtorr-msft commented Oct 24, 2023

afbjorklund commented Oct 24, 2023 • edited Loading

matthewtorr-msft commented Oct 24, 2023

afbjorklund commented Oct 24, 2023 • edited Loading

nwneisen commented Oct 24, 2023 • edited Loading

nwneisen commented Oct 24, 2023

matthewtorr-msft commented Oct 25, 2023

nwneisen commented Oct 25, 2023 • edited Loading

matthewtorr-msft commented Oct 26, 2023

nwneisen left a comment

Choose a reason for hiding this comment

matthewtorr-msft commented Oct 26, 2023

afbjorklund commented Oct 26, 2023

afbjorklund commented Oct 24, 2023 •

edited

Loading

afbjorklund commented Oct 24, 2023 •

edited

Loading

nwneisen commented Oct 24, 2023 •

edited

Loading

nwneisen commented Oct 25, 2023 •

edited

Loading