https://bugzilla.redhat.com/show_bug.cgi?id=2278420 Jerry James <loganjerry@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED --- Comment #5 from Jerry James <loganjerry@xxxxxxxxx> --- (In reply to wojnilowicz from comment #4) > Great. Thanks for your fast package review. It may take me a bit longer to > do the same, because this is my first review ever. Oh my. You picked a complicated package for your first review. :-) Note that you should change the bug status from New to Assigned when you take a bug for review. I'll do that for you. > 1) Did you went through the rpmlint list? Following warnings/errors seem > reasonable. > > python3-jupytext.noarch: W: python-leftover-require python-jupyter-filesystem This one is a false positive. The python-jupyter-filesystem package is not really a python package. It is a filesystem package, providing ownership for the directories into which the jupyter files are installed. > ppython3-jupyterlab-jupytext.noarch: E: backup-file-in-package > /usr/share/jupyter/labextensions/jupyterlab-jupytext/schemas/jupyterlab- > jupytext/package.json.orig I saw there were backup files in %{python3_sitelib} and removed those, but missed this one. I have updated the spec file to remove it, too. > 2) Is this already solved? > License file 834.90ed5e1570392532523d.js.LICENSE.txt is not marked as > %license I have updated the package to manage bundled JavaScript licenses as described here: https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/#_bundled_licenses. That won't make this warning go away, but I believe it is correct. > 3) python-jupytext-doc is provided and I believe it should be > python3-jupytext-doc. I disagree. Back in the days when we had both python 2.x and 3.x, we had packages with names like python2-foo, python3-foo, and python-foo-doc. The python3- prefix indicates the presence of python 3.x code, and the documentation packages applied to both. I think we should continue to follow that pattern in case there is a python 4 some day. There are many packages that follow this pattern already; e.g., python-CommonMark-doc, python-ansicolor-doc, python-cffi-doc, etc. I count 442 such packages currently in Fedora 40. > 4) Is this needed in Requires? The project uses pyproject.toml. > BuildRequires: %{py3_dist setuptools} > > 5) Is this needed in Requires? You don't seem to be calling npm. > BuildRequires: nodejs-npm If either one of these is removed, we get a build failure on every architecture except x86_64. I don't know why x86_64 doesn't need them. Maybe it's because I generated the vendor tarball on an x86_64 machine, so whatever step these are needed for was already done for x86_64. Traceback (most recent call last): File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module> main() File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main json_out['return_val'] = hook(**hook_input['kwargs']) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.12/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 152, in prepare_metadata_for_build_wheel whl_basename = backend.build_wheel(metadata_directory, config_settings) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.12/site-packages/hatchling/build.py", line 58, in build_wheel return os.path.basename(next(builder.build(directory=wheel_directory, versions=['standard']))) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.12/site-packages/hatchling/builders/plugin/interface.py", line 147, in build build_hook.initialize(version, build_data) File "/usr/lib/python3.12/site-packages/hatch_jupyter_builder/plugin.py", line 94, in initialize raise e File "/usr/lib/python3.12/site-packages/hatch_jupyter_builder/plugin.py", line 89, in initialize build_func(self.target_name, version, **build_kwargs) File "/usr/lib/python3.12/site-packages/hatch_jupyter_builder/utils.py", line 115, in npm_builder run([*npm_cmd, "install"], cwd=str(abs_path)) File "/usr/lib/python3.12/site-packages/hatch_jupyter_builder/utils.py", line 231, in run return subprocess.check_call(cmd, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.12/subprocess.py", line 413, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['/usr/bin/jlpm', 'install']' returned non-zero exit status 1. error: subprocess-exited-with-error > 6) There seem to be a couple of unowned directories like e.g. /etc/jupyter > and I believe that's not allowed according to > https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories/ Those are the directories owned by python-jupyter-filesystem, which is the reason this package Requires it. See bug 2264345. > 7) I still haven't looked into the package, but aren't those bundled > libraries that require the FPC exception as in > "Package contains no bundled libraries without FPC exception."? Have you > already got this exception? > # base64-js: MIT > # buffer: MIT > # ieee754: BSD-3-Clause Nodejs packages are a special case. Bundling is expected. See https://docs.fedoraproject.org/en-US/packaging-guidelines/Node.js/ I have also updated to jupytext 1.16.2, which was just released a few days ago. There is a new build in the COPR. New URLs: Spec URL: https://jjames.fedorapeople.org/python-jupytext/python-jupytext.spec SRPM URL: https://jjames.fedorapeople.org/python-jupytext/python-jupytext-1.16.2-1.fc41.src.rpm -- 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=2278420 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202278420%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
