https://bugzilla.redhat.com/show_bug.cgi?id=2022991 --- Comment #3 from Oleg Girko <ol+redhat@xxxxxxxxxxxxx> --- (In reply to Vitaly Zaitsev from comment #2) > 1. You shouldn't use macroses with double underline (%{__install}). Done. > 2. systemd units and logic should be moved to the base package. The reason for making a separate systemd subpackage is to make it optional. Not everybody wants to run i2pd as a system service, some people may prefer to run it as a regular program. I've just added "Recommends" for systemd subpackage instead. > 3. `%cmake3` should be used only on EPEL7. On Fedora and EPEL8+EPEL9 should > be used %cmake. I'd like to have a single spec file for both CentOS 7 and Fedora and avoid conditional statements as much as possible. I'm going to keep using the same spec file to build CentOS and Fedora packages in my OBS for some time. Having separate spec files for building with my OBS and with Fedora infrastructure would be confusing for me and can lead to mistakes. Using %cmake3 macro works pretty well for CentOS and across a wide range of Fedora versions, and there are no indications that this macro is going to be dropped in foreseeable future. > 4. `chrpath -d i2pd` can be dropped. Use > `-DCMAKE_SKIP_INSTALL_RPATH:BOOL=ON` instead. Done using CMAKE_SKIP_RPATH variable instead. There is no %cmake_install in %install section, all files are installed manually, no internal shared libraries used, so no need to build i2pd with rpath pointing to builddir. > 5. `%{_builddir}/%{name}-%{version}/contrib/i2pd.conf` - always use relative > paths. Done. I've reworked %install section to not cd to %{__cmake_builddir). This was needed to install just one file anyway. > 6. `Move config files from old version to proper place` - such scriplets are > not allowed. You must install everything in %install. This is needed for migration from older version of this package. Some of users may have older version installed from my OBS repo, and it would be a very unpleasant surprise for them if their configuration was not migrated to the new place automatically. Is there a better, more recommended way to migrate configuration files to a new (more standards conforming) location? > 7. Add %changelog section. There is a changelog, it's just stored in a separate file in OBS: https://obs.infoserver.lv/package/view_file/i2p/i2pd/i2pd.changes?expand=1 OBS combines spec file with changes file before building RPM package, so resulted packages have proper changelog, and the spec file included in the source RPM, contains proper changelog as well. Sorry for forgetting to mention it in this ticket's description. > 8. `Obsoletes: %{name}-daemon` - can be removed. Fedora has no package > with such name. Again, this is needed for those who upgrade a package from my OBS (although, quite an old version). > 9. `cd build` should be dropped. %cmake macro will do everything > automatically. It can't. The "build" directory is a special part of i2pd source, not a normal builddir: "CMakeLists.txt" file and cmake modules are located there. The builddir is created inside "build" directory for out of source build. > 10. `%{?cmake3_build}%{?!cmake3_build:%{make_build}}` must be replaced with > `%cmake_build`. This will break build for older Fedora versions (<31) that have no %cmake_build macro. If this is strictly necessary, I can consider dropping building for Fedora versions < 31 in my OBS and stopping supporting them, but I'd prefer to leave it as is because it has personal value for me, as an example of extremely portable spec file I've carefully crafted with as minimal changes as possible, that is still compatible with much older Fedora versions. > 11. `%{?__cmake_builddir:cd %{__cmake_builddir}}` can be dropped. I've dropped changing directory in %install section, but I still have to use another variant of this conditional macro for i2pd path, for spec file portability, as described above. -- 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 https://bugzilla.redhat.com/show_bug.cgi?id=2022991 _______________________________________________ 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
