Spec URL: https://cottsay.fedorapeople.org/python-vcs2l/python-vcs2l.spec SRPM URL: https://cottsay.fedorapeople.org/python-vcs2l/python-vcs2l-1.1.4-1.fc44.src.rpm Description: Vcs2l is a fork of Dirk Thomas's vcstool which is a version control system (VCS) tool, designed to make working with multiple repositories easier. This fork is created to continue the development of vcstool, as it is no longer actively maintained. The commands provided by vcs2l have the same naming structure as the original fork, so it can be used as a drop-in replacement. Therefore, the repository is renamed to vcs2l while maintaining the command names to vcstool to ensure compatibility with existing scripts and workflows. Fedora Account System Username: cottsay Target branches: rawhide f43 f42 epel10.1 epel10 epel9 Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=136959716 Thanks! --scott
Copr build: https://copr.fedorainfracloud.org/coprs/build/9541590 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2394287-python-vcs2l/fedora-rawhide-x86_64/09541590-python-vcs2l/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Howdy Scott, I can help with this review. Thanks for bringing it up on the epel-devel mailing list. ================================================================================ The first thing that jumps out at me is the package naming. Based on the upstream README, this seems to be primarily an application, not a library. Therefore the name of the package should be just vcs2l, not python-vcs2l. This will also allow you to remove the subpackage sections, avoid the need to create a %_description macro, and simplify %autosetup (no need to pass the -n flag). https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_application_naming ================================================================================ This package should also utilize %autorelease and %autochangelog. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_release_tag https://docs.fedoraproject.org/en-US/packaging-guidelines/#changelogs ================================================================================ I don't think you actually want to use %{python3_pkgversion} in this spec file. That is really only useful when the same package needs to be built for multiple Python versions. Since this is an application and not a library, I imagine you'll only ever want to ship a single version built against the default Python version. The spec file legibility can be improved by dropping these, e.g. python%{python3_pkgversion}-devel -> python3-devel https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_system_settings https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility ================================================================================ Currently this software just has one build requirement (pytest), but that could change in the future. Consider using the automatic buildrequirement generator so that in the future any additional build requirements get accounted for. Upstream makes this easy by defining those in a "test" extra. %pyproject_buildrequires -x test https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#Automatically-generated-dependencies ================================================================================ It isn't useful to ship shell completions in /usr/share/vcs2l-completion. These should be used to the standard locations (with standard names) so they are utilized by the respective shells. The bash, fish, and zsh completions are covered by the packaging guidelines. I don't know what the right path and naming convention is for tcsh. I'll leave it up to you if you want to figure that out or just delete that file. https://docs.fedoraproject.org/en-US/packaging-guidelines/ShellCompletions/
Carl, thanks very much for picking up the review. > Based on the upstream README, this seems to be primarily an application, not a library. At present, this package is indeed only providing an application. However, we intend to use it to revive the python-bloom package, which was retired from Fedora when "python-vcstools" (the third and oldest of these vcs utilities) was retired over a year ago due to abandonment upstream. To do that, Bloom will interact with vcs2l as a Python library. > This package should also utilize %autorelease and %autochangelog. Sure, I didn't realize this was now a recommended practice in the packaging guidelines. > Upstream makes this easy by defining those in a "test" extra. Indeed, but it required some cleanup that has yet to be released. I was planning on making the switch in the next version. https://github.com/ros-infrastructure/vcs2l/pull/63 > It isn't useful to ship shell completions in /usr/share/vcs2l-completion. That's a genuine oversight. I'll find the right spot tomorrow morning and update the review. Thanks again for taking a look! --scott
> At present, this package is indeed only providing an application. However, we intend to use it to revive the python-bloom package, which was retired from Fedora when "python-vcstools" (the third and oldest of these vcs utilities) was retired over a year ago due to abandonment upstream. To do that, Bloom will interact with vcs2l as a Python library. Thanks for the extra details about how it will be used. Regardless of the package name, it will have provides for python3dist(vcs2l) and python3.14dist(vcs2l), which should satisfy bloom's dependency as long as it's using the automatic dependency generator, so you could go a few different routes with the naming. It could just be just vcs2l (which will get the necessary provides), or python3-vcs2l (ideally providing vcs2l for discoverability). But I think for maximum correctness what I would suggest would be: * vcs2l source package * vcs2l subpackage (that depends on python3-vcs2l) with the commands and shell completions * python3-vcs2l subpackage with the library files > Indeed, but it required some cleanup that has yet to be released. I was planning on making the switch in the next version. > https://github.com/ros-infrastructure/vcs2l/pull/63 Ah, my mistake, I was viewing that test extra from the main branch where that PR is already merged. This recommendation can be deferred until the next version, or you could pull in that PR as a patch to take advantage of it now. Either way is fine.
Gentle ping, any status update on this review?
Hi there, we've just wrapped up our annual development conference so this has taken a back seat. I should be able to drive things forward this week. Thanks for your patience.
Alright, sorry for the lengthy delay. I'm back at this. We have all of the tests running now, but there are a couple of patches merged upstream that haven't been released yet. I noted that in the spec. I installed the shell completions to the right spots, and added a 'Provides' for 'vcs2l' explicitly. Spec URL: https://cottsay.fedorapeople.org/python-vcs2l/python-vcs2l.spec SRPM URL: https://cottsay.fedorapeople.org/python-vcs2l/python-vcs2l-1.1.6-1.fc44.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/9775473 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2394287-vcs2l/fedora-rawhide-x86_64/09775473-python-vcs2l/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Looking pretty good. Thanks especially for working with the upstream to get the tests working offline and for Python 3.14 compatibility. There are two more important things to fix, but they're small and you can fix them while importing the package. * You switched to %autochangelog, but missed %autorelease. * The guidelines require using the completion directory macros like %{bash_completions_dir} instead of hardcoded paths. Package is APPROVED. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [-]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 21163 bytes in 3 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: The License field must be a valid SPDX expression. [x]: Package requires other packages for directories it uses. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local Python: [x]: Python eggs must not download any dependencies during the build process. [x]: A package which is used by another package via an egg interface should provide egg info. [x]: Package meets the Packaging Guidelines::Python [x]: Package contains BR: python2-devel or python3-devel [x]: Packages MUST NOT have dependencies (either build-time or runtime) on packages named with the unversioned python- prefix unless no properly versioned package exists. Dependencies on Python packages instead MUST use names beginning with python2- or python3- as appropriate. [x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files [x]: Binary eggs must be removed in %prep ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Spec file according to URL is the same as in SRPM. [x]: Rpmlint is run on all installed packages.
The Pagure repository was created at https://src.fedoraproject.org/rpms/python-vcs2l
FEDORA-EPEL-2025-454b4ae51e (python-vcs2l-1.1.6-1.el10_1) has been submitted as an update to Fedora EPEL 10.1. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-454b4ae51e
FEDORA-EPEL-2025-a443f460a6 (python-vcs2l-1.1.6-1.el9) has been submitted as an update to Fedora EPEL 9. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-a443f460a6
FEDORA-2025-766670fee7 (python-vcs2l-1.1.6-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2025-766670fee7
FEDORA-2025-b2360dbe1c (python-vcs2l-1.1.6-1.fc43) has been submitted as an update to Fedora 43. https://bodhi.fedoraproject.org/updates/FEDORA-2025-b2360dbe1c
FEDORA-2025-766670fee7 has been pushed to the Fedora 42 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-766670fee7 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-766670fee7 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2025-13249faa41 has been pushed to the Fedora EPEL 10.2 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-13249faa41 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2025-454b4ae51e has been pushed to the Fedora EPEL 10.1 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-454b4ae51e See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2025-a443f460a6 has been pushed to the Fedora EPEL 9 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2025-a443f460a6 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2025-b2360dbe1c has been pushed to the Fedora 43 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2025-b2360dbe1c \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2025-b2360dbe1c See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2025-766670fee7 (python-vcs2l-1.1.6-1.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2025-b2360dbe1c (python-vcs2l-1.1.6-1.fc43) has been pushed to the Fedora 43 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2025-13249faa41 (python-vcs2l-1.1.6-1.el10_2) has been pushed to the Fedora EPEL 10.2 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2025-454b4ae51e (python-vcs2l-1.1.6-1.el10_1) has been pushed to the Fedora EPEL 10.1 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2025-a443f460a6 (python-vcs2l-1.1.6-1.el9) has been pushed to the Fedora EPEL 9 stable repository. If problem still persists, please make note of it in this bug report.