https://bugzilla.redhat.com/show_bug.cgi?id=2272258 --- Comment #5 from Maxwell G <maxwell@xxxxxxx> --- This is the first package to use https://gitlab.com/fedora/sigs/go/go-vendor-tools, the new tooling for vendoring Go packages, and is actually an optional dependency of go-vendor-tools itself, so there is still some work to do. See the discussion in https://lists.fedoraproject.org/archives/list/golang@xxxxxxxxxxxxxxxxxxxxxxx/thread/K5P6P2MGEE3SCPF4SZFWOIUGHQHJ6GGG/. I apologize for missing some context with this review request. I had expected for a Go SIG member who had participated in the previous discussions to review the package, but your review is very welcome. Thank you! trivy has a lot of dependencies, some of which do atypical things (e.g., the modernc dependencies), and is not representative of the average Go project. It was likely not the best first package… Anyways, I will respond to the rest of your feedback inline. (In reply to Jerry James from comment #2) > There don't seem to be any golang packaging guidelines These do exist in https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/, but don't cover the new tooling yet. > I'm doing my best to understand and review properly below. Please excuse me > if I make an ignorant comment. The review is so long that bugzilla won't let > me paste it all, so I will split it across multiple comments. Sure—thank you for bearing with me. > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > > Issues: > ======= > > - There is an awful lot of bundling going on. Is that typical for the golang > ecosystem? See the above comment about bundling. > - I am attempting to see if the License field in the spec file matches the > actual licenses in play. It's a bit of a challenge. There is no comment > in > the spec file nor any kind of README describing the license breakdown. > That > would help a lot. See the following questions. go-vendor-tools automatically computes the license tag, but there definitely should be a comment explaining that. I opened https://gitlab.com/fedora/sigs/go/go2rpm/-/issues/41. > - Many files under vendor/modernc.org/libc contain one or both of > LGPL-2.1-or-later and GPL-3.0-or-later declarations, but I don't see either > license in the License field. Should they appear there? The licensing of that project is murky. See https://gitlab.com/cznic/libc/-/issues/31. It's pulled in via trivy's dependency on modernc.org/sqlite. I wonder if upstream would consider a different sqlite driver. I'll keep digging into it. > - vendor/github.com/rcrowley/go-metrics/LICENSE is BSD-2-Clause-Views, not > BSD-2-Clause, but I don't see that in License. Fixed locally. > - Some files additionally have lines that read: > // SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note > I don't know if we are obligated to list notes, or if we only worry about > exceptions in Fedora. The files: > - vendor/modernc.org/libc/sys/socket/socket_linux_arm.go > - vendor/modernc.org/libc/sys/socket/socket_linux_arm64.go > - vendor/modernc.org/libc/sys/socket/socket_linux_riscv64.go See above. > - What do you make of the license declaration at the top of > vendor/golang.org/x/crypto/chacha20/chacha_ppc64le.s? Is that file > included > in the build on ppc64le? The link in that comment is dead, but https://web.archive.org/web/20240111224133/https://www.openssl.org/~appro/cryptogams/ still has it and https://github.com/dot-asm/cryptogams/blob/a60f5b50ed908e91e5c39ca79126a4a876d5d8ff/LICENSE suggests that this is available under the BSD-3-Clause license OR (an unspecified version of) the GPL. golang-x-crypto is licensed under BSD-3-Clause as well and only retains that notice. go-vendor-tools already detects this dependency as BSD-3-Clause, so we should be okay there. I don't think adding an "OR GPL-1.0-or-later" to account for > ALTERNATIVELY, provided that this notice is retained in full, this product may be distributed under the terms of the GNU General Public License (GPL), in which case the provisions of the GPL apply INSTEAD OF those given above. in the original project's license makes sense here. > - vendor/github.com/alecthomas/chroma/formatters/svg/font_liberation_mono.go > contains an encoding of a font under the OFL-1.1-RFN license, which does > not > appear in License. Fixed locally. > - See the complaint below about unowned directories. This is not an error. > The directory /usr/share/licenses/trivy/vendor/github.com/kylelemons, for > example, is not owned by this package, but contains another directory that > is. Tracked at https://gitlab.com/fedora/sigs/go/go-vendor-tools/-/issues/44. The code that generates the license filelist also needs to add entries for intermediate directories. > - See the non-executable-script rpmlint warnings below. Please either remove > the shebangs from those files or make them executable. > - Notice the invalid-url rpmlint warning for Source0. The URL is missing > "https:" at the beginning. Is this a weakness of %gourl? Is something > missing from the spec file that would cause that to appear? This seems to be a false positive. The URL is expanded properly when you evaluate the specfile. > - Note that unused-direct-shlib-dependency warning for /usr/bin/trivy. It > depends, uselessly, on libresolv.so.2. Does that mean /usr/bin/trivy was > linked without --as-needed? > - Version 0.50.1 has been released, FYI. Thanks for the heads up. I'll take a look at updating after I've addressed the other outstanding tooling issues. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2272258 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202272258%23c5 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
