https://bugzilla.redhat.com/show_bug.cgi?id=1983601 Tomas Korbar <tkorbar@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(tkorbar@xxxxxxxxx | |m) | --- Comment #3 from Tomas Korbar <tkorbar@xxxxxxxxxx> --- Hi Christopher, thanks for your review. I think i managed to address all of the issues One question from my side: Is /usr/lib64/guile/3.0/extensions/guile-readline.so* something that is supposed to be generally available or is that guile-internal like the stuff in .../ccache ? ----- Internal IMO, it is guiles module Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= - [MUST] If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. Please add BuildRequires: gcc ----- Done - [MUST] There are some directories that should be owned by this package but aren't: /usr/share/guile/site/3.0, /usr/share/guile/3.0/scheme, /usr/share/guile/site Please add them to %files ----- Done - [MUST] Texinfo files are installed using install-info in %post and %preun if package has .info files. ----- Done. - [MUST] Some parts of the code seem to be MIT/GPL/PublicDomain licensed, rather than LGPL, please check if these end up in the package MIT License: - guile-3.0.7/doc/ref/sxml-match.texi - guile-3.0.7/module/ice-9/quasisyntax.scm - guile-3.0.7/module/srfi/srfi-38.scm - guile-3.0.7/module/srfi/srfi-41.scm - guile-3.0.7/module/srfi/srfi-45.scm - guile-3.0.7/module/srfi/srfi-64/testing.scm - guile-3.0.7/module/srfi/srfi-67/compare.scm - guile-3.0.7/module/srfi/srfi-71.scm - guile-3.0.7/module/sxml/sxml-match.ss Public domain - guile-3.0.7/module/ice-9/match.upstream.scm GPL - guile-3.0.7/guile-readline/ice-9/readline.scm GPLv3+ - guile-3.0.7/guile-readline/readline.c - guile-3.0.7/guile-readline/readline.h - guile-3.0.7/libguile/libguile-3.0-gdb.scm - guile-3.0.7/module/language/elisp/compile-tree-il.scm ----- These files are included so i added licenses. It is unfortunate - [SHOULD] Please add dependencies on pkg-config and change (build)requires to pkgconfig(<libraryname>). See https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/ ----- Done - [SHOULD] Please add comment with the rationale for the bundled provide, e.g. that guile ships a patched version of a git checkout of localcharset.[h,c] from gnulib, as intended by gnulib upstream. ----- Done - [SHOULD] Please also add the version of the bundled gnulib. Brief look at upstream seems to indicate that it is v0.1-1157-gb03f418, but that doesn't really line up with the numbering scheme the fedora package uses (just Version: 0 coupled to a Release: with git suffix) or with the way gnulib works in general I'd suggest just adding the Version as a comment. Git history of the file is here: https://git.savannah.gnu.org/gitweb/?p=guile.git;a=commit;h=a91b95cca2d397c84f8b9bbd602d40209a7092ce ----- Done - [SHOULD] Please list libguile version explicitly in %files, rather than via glob (i.e as libguile*.so..X.Y.Z or at least .X*) to protect against accidental library version bumps. See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shared_libraries ----- Done - [SHOULD] Please add brief comments and/or upstream links to patches to explain what they do. ----- Done - [SHOULD] Doesn't build on aarch64 due to test fail: error: 'guild compile' failed to remove 't-guild-compile-100826.go.64hinA' FAIL: test-guild-compile wrote `/builddir/build/BUILD/guile-3.0.7/cache/guile/ccache/3.0-LE-8-4.5/builddir/build/BUILD/guile-3.0.7/test-suite/standalone/test-signal-fork.go' parent: child: 100919101058 ----- I did not encounter any such issue during scratch builds. It seems that some of the tests are not really stable. If this test will keep failing in the future, then i will disable it. - [EXTRA] [RPMLINT] [PATCH] missing-call-to-chdir-with-chroot is most likely triggered by libguile/posix.c, and for once seems to me to not be a false positive. Please check (I'm not that much of a C expert) and consider adding a patch that calls chdir() before chrooting, as this is a potential security issue. ----- Done - [EXTRA] [PATCH] configure.ac contains AC_PROG_LIBTOOL, which is deprecated. Maybe add a patch to replace it with LT_INIT? No further changes should be necessary. See https://www.gnu.org/software/libtool/manual/html_node/LT_005fINIT.html ----- Done - [EXTRA] [RPMLINT] [PATCH] Fix FSF address. ----- Upstream has been notified but i am not comfortable with downstream patching license. - [NON-ISSUE] Further (informative, non-issue) comments inline below, enclosed in ****. Updated specfile and srpm are in the same place, the originals were. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
