https://bugzilla.redhat.com/show_bug.cgi?id=2521853
--- Comment #4 from Fabio Valentini <[email protected]> --- (In reply to Ben Beasley from comment #3) > Package Review > ============== Thanks for taking a look! > ===== Issues ==== > > - The LICENSE.dependencies file is not included in the binary RPMs. Thanks for the suggestion, I have moved %pyproject_wheel to the bottom of %build. > - In %check, you might consider adding a comment that all tests currently > require network access. Good point, I can add a comment to that effect. Note that there *would* be at least "doctests", but they fail to compile with linker errors on ppc64le. I don't know *why* libpython.so symbols don't get linked to the test executable *only on this architecture*, but there are a handful of open and closed issues in the pyo3 issue tracker about "undefined reference" issues when running "cargo test" for a PyO3 cdylib. Weirdly enough, "cargo test" does compile and run the doctests, but "cargo test --doc" aborts with an error message claiming that "doctests can't be run for cdylib targets". :confused: > ===== Notes (no change required for approval) ===== > > - You may, if you like, remove the explicit BuildRequires: python3-devel, > since > %pyproject_buildrequires suffices. See > https://pagure.io/packaging-committee/pull-request/1379/ and > https://pagure.io/packaging-committee/pull-request/1384/. The explicit > BuildRequires is not incorrect and does not hurt anything. > > - I generally feel that defining pypi_name, srcname, modname, or similar > macros > just adds noise to the spec file and makes it harder to read for little or > no > benefit. I favor simply writing out the name in question. There is no rule > about this, and you are free to keep the macro indirection if you like it. I would agree in general, though in this case, it appears that I need to specify the "normalized" PyPI name with "_" as separator at least in two places - in the %pypi_source macro argument and in the %pyproject_save_files macro argument. Repeating the name twice wouldn't be that bad (especially since it won't change without a package name / upstream name change) though, I guess. > - rpmlint notes: > > python3-crate-ingredients.x86_64: W: > crypto-policy-non-compliance-openssl > /usr/lib64/python3.15/site-packages/crate_ingredients/_rust.cpython-315- > x86_64-linux-gnu.so SSL_CTX_set_cipher_list > > This is tricky, because this API call isn’t directly in the package’s > source > code. The symbol SSL_CTX_set_cipher_list is linked from the shared library, > but we aren’t able to easily determine whether or when it is actually > called, > or with what parameters. As noted in > > https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/ > #_cc_applications, > calling it with "PROFILE=SYSTEM" would be just fine. I used the strings > tool > on the compiled Python extension and couldn’t find either PROFILE=SYSTEM or > anything that looked like a hard-coded cipher list. > > It would be nice to figure out whether this is really correct or not, but > at > least I think we can say that *if* there is a problem, then the fix is > almost > certainly in one of this package’s crate dependencies rather than in this > package itself. As far as I can tell, this is basically always a false positive. There aren't any code paths (that I have found) from reqwest -> native-tls -> openssl that call SSL_CTX_set_cipher_list. And applications that use the default SSL context from the "openssl" crate use PROFILE=SYSTEM: https://src.fedoraproject.org/rpms/rust-openssl/blob/rawhide/f/0001-set-PROFILE-SYSTEM-as-default-cipher-list.patch > [x]: Requires correct, justified where necessary. > > It’s fairly obvious that the explicit Requires on cargo and git-core are > because these are used by calling their command-line tools. I don’t > think > these require additional documentation. I'll add a comment to the .spec file. The README documents that they're hard requirements, but documenting it in the .spec file too won't hurt. -- 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=2521853 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202521853%23c4 -- _______________________________________________ package-review mailing list -- [email protected] To unsubscribe send an email to [email protected] 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/[email protected] Do not reply to spam, report it: https://forge.fedoraproject.org/infra/tickets/issues/new
