Bug 2188762
| Summary: | Review Request: golang-github-containerd-cgroups-3 - Cgroups package for Go | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Sergio Basto <sergio> |
| Component: | Package Review | Assignee: | Robert-André Mauchin 🐧 <zebob.m> |
| Status: | ASSIGNED --- | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | mikel, package-review, zebob.m |
| Target Milestone: | --- | Flags: | zebob.m:
fedora-review?
zebob.m: needinfo? (sergio) |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | Type: | --- | |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Sergio Basto
2023-04-22 03:58:51 UTC
This package built on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=100241683 > Epoch: 1 Why epoch is needed for this new package? > Release: 3%{?dist} Why it doesn't start from 1? But also, why are you not using %autorelease? > %build > mkdir v3 > mv cmd/go.mod cmd/go.sum v3 > for cmd in cmd/* ; do > %gobuild -o %{gobuilddir}/bin/$(basename $cmd) %{goipath}/$cmd > done (...) > install -m 0755 -vd %{buildroot}%{_bindir} > install -m 0755 -vp %{gobuilddir}/bin/* %{buildroot}%{_bindir}/ Is the cmd useful? (In reply to Mikel Olasagasti Uranga from comment #2) > > Epoch: 1 > > Why epoch is needed for this new package? > because golang-github-containerd-cgroups also have epoch = 1 and golang-github-containerd-cgroups-3 need be bigger than golang-github-containerd-cgroups > > Release: 3%{?dist} > > Why it doesn't start from 1? > > But also, why are you not using %autorelease? > fixed > > %build > > mkdir v3 > > mv cmd/go.mod cmd/go.sum v3 > > for cmd in cmd/* ; do > > %gobuild -o %{gobuilddir}/bin/$(basename $cmd) %{goipath}/$cmd > > done > (...) > > install -m 0755 -vd %{buildroot}%{_bindir} > > install -m 0755 -vp %{gobuilddir}/bin/* %{buildroot}%{_bindir}/ > > Is the cmd useful? if we can build it and provides the binary /usr/bin/cgctl , why not ? golang-github-containerd-cgroups doesn't provides it, but we need a trick to build it, we need to move cmd/go.mod cmd/go.sum away from the directory cmd [1] [1] mkdir v3 mv cmd/go.mod cmd/go.sum v3 >> Why epoch is needed for this new package? > because golang-github-containerd-cgroups also have epoch = 1 and golang-github-containerd-cgroups-3 need be bigger than golang-github-containerd-cgroups In the case of `golang-github-containerd-cgroups` it is required because a newer version was pushed and then restored the old one. For `golang-github-containerd-cgroups-3`, the package name is different (has the -3) and goipath is also different. I don't see it as required. > %global goipathsex0 github.com/containerd/cgroups/v3/cmd Why is this one required? Builds fine for me without it. > # Needs special permissions for cgroupsv2 > %bcond_with check Skip those tests failing, not all the tests. You can check how `doctl.spec` does for example to skip just some tests. > RPM build warnings: > File listed twice: /usr/share/doc/golang-github-containerd-cgroups-3/metrics.pb.txt > File listed twice: /usr/share/doc/golang-github-containerd-cgroups-3-devel/metrics.pb.txt You can check how to fix this one using `golang-github-cloudflare-circl.spec` LICENSE rename as example. - I removed the Epoch - enabled teste and removed "TestCgroupv2CpuStats" "TestSystemdCgroupCpuController" "TestSystemdCgroupCpuController_NilWeight" "TestEventChanCleanupOnCgroupRemoval"\ "TestCgroupv2MemoryStats" "TestSystemdCgroupMemoryController" "TestCgroupv2PidsStats" "TestSystemdCgroupPidsController" "TestKill" because needs special permissions - Fixed the File listed twice like was in golang-github-cloudflare-circl.spec about goipathsex0 in golang channel me and gotmax23 talk: sergiomb: the problem of golang-github-containerd-cgroups-3 is have cmd/go.mod cmd/go.sum which aren't meant to bulld mkdir v3 mv cmd/go.mod cmd/go.sum v3 seems thats fixes the problem gotmax23G :That won't work either sergiomb: can you explain ? where it will fail gotmax23: The cmd directory needs to be outside of the v3 directory. (later) gotmax23: Hmm, it might be possible to regenerate a clean specfile with go2rpm --forge https://github.com/containerd/cgroups github.com/containerd/cgroups/v3 and then add %global goipathsex0 github.com/containerd/cgroups/v3/cmd right under %gometa. (later) sergiomb: after %gometa %global goipathsex0 github.com/containerd/cgroups/v3/cmd makes disappear cmd at all gotmax23: That's the point. Since no other package should depend on the cmd module, it might be best to just exclude it rather than install it at the wrong location. new spec and src.rpm uploaded I have my own SPEC going on in #2229425.
Anyhow
- Bump to 3.0.2
- Are these files needed? For what purpose are they in docs?
cgroup2-stats-metrics.pb.txt cgroup1-stats-metrics.pb.txt
- What is the purpose of:
mkdir v3
mv cmd/go.mod cmd/go.sum v3
Just do:
%build
%gobuild -o %{gobuilddir}/bin/cgctl %{goipath}/cmd/cgctl
- Format this:
for test in "TestCgroupv2CpuStats" \
"TestSystemdCgroupCpuController" \
"TestSystemdCgroupCpuController_NilWeight" \
"TestEventChanCleanupOnCgroupRemoval" \
"TestCgroupv2MemoryStats" \
"TestSystemdCgroupMemoryController" \
"TestCgroupv2PidsStats" \
"TestSystemdCgroupPidsController" \
"TestKill" \
"TestMoveTo" ; do
awk -i inplace '/^func.*'"$test"'\(/ { print; print "\tt.Skip(\"disabled failing test\")"; next}1' $(grep -rl $test)
done
- Don't glob the binary directory:
%files
%license LICENSE
%doc README.md
%{_bindir}/cgctl
- > sergiomb: the problem of golang-github-containerd-cgroups-3 is have cmd/go.mod cmd/go.sum which aren't meant to bulld
This is really not needed, Fedora Go macros don't use Go modules. You can remove it.
Please update the SPEC and we'll get back to it.
*** Bug 2229425 has been marked as a duplicate of this bug. *** |