Spec URL: https://github.com/timothysc/containernetworking/blob/master/containernetworking.spec SRPM URL: https://github.com/timothysc/containernetworking/raw/master/containernetworking-0.5.1-1.fc27.src.rpm Description: The CNI (Container Network Interface) project consists of a specification and libraries for writing plugins to configure network interfaces in Linux containers, along with a number of supported plugins. CNI concerns itself only with network connectivity of containers and removing allocated resources when the container is deleted. Fedora Account System Username: tstclair
Given the cni is a go project: - if it is possible we should package entire devel subpackage (including its dependencies) - if it is possible the project should be built from bundled dependencies - if possible, project tests should be run - if possible, the project should be built over all available architectures - if possible, the spec file header should contain macros that reference project location I have generated the new spec file via gofed and incorporated part of your spec file into it [1]. Currently (referring to your spec file): - project is built from bundled dependencies (I would recommended that only in cases when there is no other way) - devel subpackage is missing build-time and run-time dependencies - tests are not run (given some of the tests need to be run as a root and others need additional configuration, they do not have to be run) - no unit-test subpackage: even if the tests are not run, the _test.go files can by analysed by tooling and/or run in specialized CI so it is always useful to package the unit-test package as well - though there is no strict structure of the spec file header, I recommend to define provider_prefix, import_path and commit macros. I got a tooling that periodically scans all go projects in Fedora rawhide and collects various data/metadata from the go rpms. Plus, if the macros are set, you can use gofed tooling to update the spec file. - exclusive archs are set to %{go_arches}: out aim is to generate spec files that can be built over various architectures and OSes. Unfortunately, the %{go_arches} macro is not present on CentOS or RHEL so the current spec file with work on Fedora only. Check [1] for the general ExclusiveArch value. The same holds for golang BuildRequires. - in Fedora %gobuild macro is defined that encompasses the BUILD_ID for you so there is no need to do that manually. However, the same limitations hold here. The macro is present on Fedora only so the macro is redefine at spec file header if not defined, see [1]. - I would not call the ./build script as it usually runs more than is needed and hides the building itself. As long as the build steps are simple, I would recommend to put them into the %build section. - not sure about the name, kubernetes uses kubernetes-cni, which I would see instead of containernetworking. But, I have no strong preference over the name so basically any name that is close to cni is acceptable. The references spec file [1] is an example, not something you need to follow. Yeah, it is pretty long and there is a lot of constructions that make it hard to read. But, it has its advantages. [1] https://github.com/gofed/reviews/pull/5/files
Some of the auto-generated portions make my eyes sore, I'll merge the two and repost today.
I just cleaned up the spec that you had sent, I don't have strong opinions on the format, there are minor comments in the .spec file.
> # Obsoletes: kubernetes-cni There is no kubernetes-cni in Fedora so far so no need to obsolete one > # I think this is brittle vs. the build tooling as it will Other option is to patch ./build to use gcc-go in case some architectures requires it. I agree that every time the package is updated the list needs to be updated as well. Another option is to copy: ```sh PLUGINS="plugins/meta/* plugins/main/* plugins/ipam/* plugins/test/*" for d in $PLUGINS; do if [ -d $d ]; then plugin=$(basename $d) echo " " $plugin %gobuild -o bin/$plugin %{import_path}/$d fi done ``` and replace all the %gobuild -o bin/plugins/ like lines. > # why can't you go test ./... Very often some of the tests fail due to incomplete environment configuration. Given the list the failing tests can be commented out to run at least something. At the same time the failing tests can be commented with a reason why they are failing. Once a CI take care of the tests, the %check section will disappear. > Name: containernetworking Given the project import path prefix is github.com/containernetworking/cni, I would stick with containernetworking-cni name in case other projects will appear under github.com/containernetworking. In future, they may be github.com/somecompany/containernetworking project which would collide with the name.
Updates: - Updated name per comment - Moved some comments as TODO to cleanup later. Spec URL: https://raw.githubusercontent.com/timothysc/containernetworking-cni/master/containernetworking-cni.spec SRPM URL: https://github.com/timothysc/containernetworking-cni/raw/master/containernetworking-cni-0.5.1-1.fc27.src.rpm
Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=19609820 $ rpmlint containernetworking-cni-debuginfo-0.5.1-1.fc24.x86_64.rpm containernetworking-cni-unit-test-devel-0.5.1-1.fc24.x86_64.rpm containernetworking-cni-devel-0.5.1-1.fc24.noarch.rpm containernetworking-cni-0.5.1-1.fc24.x86_64.rpm containernetworking-cni-0.5.1-1.fc24.src.rpm containernetworking-cni-unit-test-devel.x86_64: W: spelling-error %description -l en_US github -> git hub, git-hub, GitHub containernetworking-cni-devel.noarch: W: spelling-error %description -l en_US github -> git hub, git-hub, GitHub containernetworking-cni.x86_64: W: no-manual-page-for-binary cnitool 5 packages and 0 specfiles checked; 0 errors, 3 warnings. LGTM, approving
Looks like the process has changed... but I'll still drop this here. New Package SCM Request ======================= Package Name: containernetworking-cni Short Description: Libraries for writing CNI plugin Owners: tstclair, jchaloup Branches: f26, epel7
This package was approved and imported in repositories and it was later retired, but this review ticket was never closed. I'm closing it now.