https://bugzilla.redhat.com/show_bug.cgi?id=1565848
Bug ID: 1565848 Summary: Review Request: Bear - Tool that generates a compilation database for clang tooling Product: Fedora Version: rawhide Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: dan.cermak@cgc-instruments.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/defolos/devel/Bear.git/tree/b... SRPM URL: https://copr-be.cloud.fedoraproject.org/results/defolos/devel/fedora-27-ppc6...
Description: This is my first package, therefore I need a sponsor.
Bear (=Build ear) produces a compilation database in the JSON format. This database describes how single compilation unit should be processed and can be used by Clang tooling. This can be used to generate the JSON compilation database when using build systems that cannot create it themselves.
Koji scratch builds: el7: https://koji.fedoraproject.org/koji/taskinfo?taskID=26296400 el6: https://koji.fedoraproject.org/koji/taskinfo?taskID=26296413 f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=26296506 f28: https://koji.fedoraproject.org/koji/taskinfo?taskID=26296521 f29: https://koji.fedoraproject.org/koji/taskinfo?taskID=26296559
Copr repo: https://copr.fedorainfracloud.org/coprs/defolos/devel/
The spec file was originally written by Pavel Odvody for Bear 2.1.2, I have modified it so that it works with the most recent version of Bear.
Fedora Account System Username: defolos
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
dan.cermak@cgc-instruments.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
Iwicki Artur fedora@svgames.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@svgames.pl
--- Comment #1 from Iwicki Artur fedora@svgames.pl ---
URL: https://github.com/rizsotto/%%7Bname%7D Source: https://github.com/rizsotto/%%7Bname%7D/archive/%%7Bversion%7D.tar.gz
Don't repeat yourself; use %{URL} as part of Source.
BuildRequires: cmake make python gcc Requires: python
With Python2 being slowly phased out, I believe you should use an explicit "python2" or "python3" requires.
$ head -n1 /usr/bin/bear #!/usr/bin/env python
"/usr/bin/env" MUST NOT be used. Edit this line with awk, or through a Patch, and make it use "/usr/bin/python{2,3}". https://fedoraproject.org/wiki/Packaging:Guidelines#Shebang_lines
%doc %{_mandir}/man1/bear.1*
Man pages should not be marked as %doc.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #2 from dan.cermak@cgc-instruments.com --- Thank you for the swift review!
I have addressed your comments and fixed the issues. I have also uploaded the spec & patch file to pagure: https://pagure.io/Bear_rpm and pushed the new source rpm to Copr: https://copr.fedorainfracloud.org/coprs/defolos/devel/build/739812/
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #3 from dan.cermak@cgc-instruments.com --- My apologies, I have forgotten one unversioned python in the spec file. The Copr build without that is: https://copr.fedorainfracloud.org/coprs/defolos/devel/build/739830/
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #4 from dan.cermak@cgc-instruments.com --- And now the CentOS build failed. I'll report back when I have fixed it.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #5 from dan.cermak@cgc-instruments.com --- I have fixed the problem on EL, the new spec file now builds successfully on Copr (https://copr.fedorainfracloud.org/coprs/defolos/devel/build/740012/).
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
Darryl T. Agostinelli dagostinelli@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dagostinelli@gmail.com
--- Comment #6 from Darryl T. Agostinelli dagostinelli@gmail.com --- This is an informal review
- fedora-review fails locally
You should update the SPEC and SRPM links in this bugzilla bug so that fedora-review can work automatically. It's a very helpful tool.
$ fedora-review -b 1565848 INFO: Processing bugzilla bug: 1565848 INFO: Getting .spec and .srpm Urls from : 1565848 INFO: --> SRPM url: https://copr-be.cloud.fedoraproject.org/results/defolos/devel/fedora-27-ppc6... INFO: --> Spec url: http://copr-dist-git.fedorainfracloud.org/cgit/defolos/devel/Bear.git/tree/b... INFO: Using review directory: /home/dagostinelli/projects/fedora/1565848-bear INFO: Downloading .spec and .srpm files ERROR: 'Error 404 downloading https://copr-be.cloud.fedoraproject.org/results/defolos/devel/fedora-27-ppc6...' (logs in /home/dagostinelli/.cache/fedora-review.log)
- package names SHOULD be lowercase
Package names SHOULD be in lower case and use dashes in preference to
underscores.
https://fedoraproject.org/wiki/Packaging:Naming
Consider just naming your package "bear".
- Possible name conflict
Are you affiliated with the existing package called "bear-devel"?
`dnf info bear-devel`
See: https://github.com/j-jorge/bear
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #7 from dan.cermak@cgc-instruments.com --- Thanks for the informal review!
Unfortunately I don't know how to edit the original bugreport. Is it sufficient to include the url in a later message?
Otherwise, I have updated the rpm spec file on pagure and changed the name to lowercase only. It builds successfully on copr: https://copr.fedorainfracloud.org/coprs/defolos/devel/build/755412/ (not on rawhide, but the issue there is unrelated to my spec file, the rawhide repos look broken on the builder).
Concerning bear-devel: no I am neither affiliated with that, nor has it anything to do with this package. bear-devel is a game engine, bear is a clang tooling helper to generate the json compilation database by hooking into an existing build system. Unfortunately the upstream names are both bear.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #8 from Darryl T. Agostinelli dagostinelli@gmail.com --- - Spec and SRPM URL
Create a new comment with the first two lines being the spec and srpm links, like you did the first time. The fedora-review tool is smart enough to grab the most recent comment. If you look through other review requests in bugzilla, you can find examples of this.
- Conflicting name
https://fedoraproject.org/wiki/Packaging:Conflicts#Conflicting_Package_Names
This may get your package denied.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #9 from dan.cermak@cgc-instruments.com --- Spec URL: https://pagure.io/Bear_rpm/raw/master/f/bear.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/defolos/devel/fedora-28-x86_...
- Conflicting name
Well the names are not actually the same, although they are confusingly similar and one would expect that bear-devel was the development package for bear. Tbh it would be the best if bear-devel was called bear-engine-devel, but it's probably too late for that to happen. Maybe I can rename the package to its full name: build-ear.
Should I ask upstream & the packaging committee for approval?
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
dan.cermak@cgc-instruments.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |1546544
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1546544 [Bug 1546544] Re-Review Request: renaming package bear to bear-factory
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #10 from Darryl T. Agostinelli dagostinelli@gmail.com --- I couldn't say what the best name would be, though naming it build-ear does sound appropriate to me. Yourself or upstream would know best.
Renaming the existing project would involve working with the other maintainer and following the package renaming process. After all that work, I'm not sure if you can then pickup the name bear at the end of that. It certainly isn't a quick process.
https://fedoraproject.org/wiki/Package_Renaming_Process
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zebob.m@gmail.com
--- Comment #11 from Robert-André Mauchin zebob.m@gmail.com --- Please rename the package, bear-clang, build-ear, choose what you want.
Then update this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #12 from dan.cermak@cgc-instruments.com --- The conflicting package bear has been renamed to bear-factory (https://bugzilla.redhat.com/show_bug.cgi?id=1546544).
I'd therefore keep the name, if that is unambiguous enough. I have also asked upstream and the author would also be fine with calling it build-ear.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
Till Hofmann thofmann@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |thofmann@fedoraproject.org Depends On|1546544 |1539207
--- Comment #13 from Till Hofmann thofmann@fedoraproject.org --- The current bear package has been renamed, so this can now keep the name 'bear'.
I would have preferred if you'd taken part in the ongoing effort to rename and package instead of just submitting your own review request without any prior coordination. But it's done now.
If you can find a sponsor, I'd be happy to see this finished. Alternatively, I have a prepared SPEC here that I could submit for review. I'd be happy to have you as a co-maintainer, if we go down that road.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1539207 [Bug 1539207] Rename to bear-factory https://bugzilla.redhat.com/show_bug.cgi?id=1546544 [Bug 1546544] Review Request: bear-factory - renaming the bear package
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #14 from Darryl T. Agostinelli dagostinelli@gmail.com ---
I'd therefore keep the name, if that is unambiguous enough. I have also asked upstream and the author would also be fine with calling it build-ear.
Please use build-ear then.
I think we can all imagine a future scenario where another project come along wanting to use the name "bear" and then this whole thing repeats itself.
Since you say that upstream is fine with build-ear; then go with build-ear.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #15 from Till Hofmann thofmann@fedoraproject.org --- I disagree. As pointed out on the mailing list and in #1539207, all distros other than Fedora use the name bear. The general agreement on fedora devel was to rename the current bear and use the name bear for the compilation database.
Imho, the package name should be just bear.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #16 from Darryl T. Agostinelli dagostinelli@gmail.com ---
As pointed out on the mailing list and in #1539207, all distros other than Fedora use the name bear. The general agreement on fedora devel was to rename the current bear and use the name bear for the compilation database.
Thank you. I retract my recommendation. Keep the name as bear.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |zebob.m@gmail.com Summary|Review Request: Bear - Tool |Review Request: bear - Tool |that generates a |that generates a |compilation database for |compilation database for |clang tooling |clang tooling Flags| |fedora-review?
--- Comment #17 from Robert-André Mauchin zebob.m@gmail.com --- - Use a more meaningful name for your package:
Source: %{URL}/archive/%{version}/%{name}-%{version}.tar.gz
- Not needed:
Requires: python%{python3_pkgversion}
It should be picked up automatically
- Split you BR and R to be one per line
- sed and make are not needed as BR as they are in the default buildroot
- BuildRequires: python%{python3_pkgversion} not needed, it is implied by -devel
- Not needed anymore and not needed for a private library anyway:
%ldconfig_post
%ldconfig_postun
- Don't use %doc here if it's already installed in the doc directory. Also remove the glob * to own the directory:
%{_datadir}/doc/bear
- You must install the license files with %license in %files
%license COPYING
- You should install the Readme and Changelog with %doc:
%doc ChangeLog.md README.md
- Own /usr/lib64/bear:
%dir %{_libdir}/bear/
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - 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. Note: License file COPYING is not marked as %license See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs.
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. Note: Checking patched sources after %prep for licenses. Licenses found: "GPL (v3 or later)", "Unknown or generated". 59 files have unknown license. Detailed output of licensecheck in /home/bob/packaging/review/bear/review-bear/licensecheck.txt [!]: License file installed when any subpackage combination is installed. [!]: Package requires other packages for directories it uses. Note: No known owner of /usr/lib64/bear, /usr/share/doc/bear [x]: %build honors applicable compiler flags or justifies otherwise. [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. [x]: 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. [x]: 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]: Useful -debuginfo package or justification otherwise. [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 61440 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: No rpmlint messages. [x]: Package does not own files or directories owned by other packages. [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 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
===== 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]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in bear- debuginfo , bear-debugsource [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [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]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: bear-2.3.13-1.fc30.x86_64.rpm bear-debuginfo-2.3.13-1.fc30.x86_64.rpm bear-debugsource-2.3.13-1.fc30.x86_64.rpm bear-2.3.13-1.fc30.src.rpm 4 packages and 0 specfiles checked; 0 errors, 0 warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #18 from dan.cermak@cgc-instruments.com ---
I would have preferred if you'd taken part in the ongoing effort to rename and package instead of just submitting your own review request without any prior coordination. But it's done now.
I am very sorry about this outcome, I have submitted this package for review a quite while ago and although I searched for packaging efforts for bear (that was however mainly limited to open review requests), I haven't found any. I only became aware of the name clash with the bear game engine after Darryl pointed it out to me. Only then did I find out about you trying to rename the current bear-* packages to something not clashing with bear.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #19 from dan.cermak@cgc-instruments.com --- Thanks for the extensive review, I have tried to address most of the issues and pushed the new version to pagure (https://pagure.io/Bear_rpm/blob/master/f/bear.spec) and copr (https://copr.fedorainfracloud.org/coprs/defolos/devel/).
However, concerning this:
- Use a more meaningful name for your package:
I'd prefer not to change the package's name, as it is widely known as bear (it is recommended frequently when working with clang-tooling) and most other distribution package it under its upstream name.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #20 from Robert-André Mauchin zebob.m@gmail.com --- (In reply to dan.cermak from comment #19)
Thanks for the extensive review, I have tried to address most of the issues and pushed the new version to pagure (https://pagure.io/Bear_rpm/blob/master/f/bear.spec) and copr (https://copr.fedorainfracloud.org/coprs/defolos/devel/).
However, concerning this:
- Use a more meaningful name for your package:
I'd prefer not to change the package's name, as it is widely known as bear (it is recommended frequently when working with clang-tooling) and most other distribution package it under its upstream name.
I'm only talking about the archive name, instead of %{version}.tar.gz use %{name}-%{version}.tar.gz. It it less ambiguous.
- Use a more meaningful name for your archive:
Source: %{URL}/archive/%{version}/%{name}-%{version}.tar.gz
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #21 from dan.cermak@cgc-instruments.com ---
I'm only talking about the archive name, instead of %{version}.tar.gz use %{name}-%{version}.tar.gz. It it less ambiguous.
- Use a more meaningful name for your archive:
Source: %{URL}/archive/%{version}/%{name}-%{version}.tar.gz
Ah, I see. Thanks for the explanation, I did not know that you could download the sources from github via this url.
I have pushed the changes to the pagure repository.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #22 from Robert-André Mauchin zebob.m@gmail.com --- - Not needed:
Requires: python%{python3_pkgversion}
It should be picked up automatically
Package approved.
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
Till Hofmann thofmann@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) |
--- Comment #23 from Till Hofmann thofmann@fedoraproject.org --- I just sponsored Dan.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #24 from Till Hofmann thofmann@fedoraproject.org --- Some remarks:
- The license should be GPLv3+, not GPLv3.
- As Robert-André pointed out, you don't need to Require python.
- The shebang replacement should keep the timestamp. Your version is also not safe, as it would substitute "/usr/bin/env python" anywhere in the file, and it would replace "/usr/bin/env python3" by "/usr/bin/python33". I usually use the following snippet in %install instead: for f in %{buildroot}/%{_bindir}/* ; do sed -i.orig "s:^#!/usr/bin/env\s+python\s?$:#!%{__python3}:" $f touch -r $f.orig $f rm $f.orig done
Note that if the shebang is "/usr/bin/env python3", then that is fine, because the mangler will automatically change it to "/usr/bin/python3", see [1].
- In the future, please follow the pattern with URLs to the SPEC and SRPM on the first two lines, as in the original post. The URLs should point to a raw SPEC and a directly downloadable SRPM. This allows a reviewer to use fedora-review.
- The file section can be simplified, e.g., instead of %dir %{_libdir}/bear/ %{_libdir}/bear/libear.so you can simply write %{_libdir}/bear Similarly for the docs.
- You list some doc files twice. If you list a file such as "%doc README.md", then you don't need to list it again. The build system already installs those files, so either you list them with %doc %{_docdir}/bear, or you remove them in %install and install them with %doc README.md etc. You also have two copies of COPYING. No need to have it in the doc dir, although that's not a big issue either.
- Why do you have conditionals on %{?fedora}?
[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Shebang_lines
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #25 from dan.cermak@cgc-instruments.com --- Spec URL: https://pagure.io/Bear_rpm/raw/master/f/bear.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/defolos/devel/fedora-29-x86_...
(In reply to Till Hofmann from comment #24)
Some remarks:
- The license should be GPLv3+, not GPLv3.
Done
- As Robert-André pointed out, you don't need to Require python.
Done
- The shebang replacement should keep the timestamp. Your version is also
not safe, as it would substitute "/usr/bin/env python" anywhere in the file, and it would replace "/usr/bin/env python3" by "/usr/bin/python33". I usually use the following snippet in %install instead: for f in %{buildroot}/%{_bindir}/* ; do sed -i.orig "s:^#!/usr/bin/env\s+python\s?$:#!%{__python3}:" $f touch -r $f.orig $f rm $f.orig done
Note that if the shebang is "/usr/bin/env python3", then that is fine, because the mangler will automatically change it to "/usr/bin/python3", see [1].
I have tried that, but the mangler chooses python2 instead and complains that it will become an error soon. I have instead opted to use your snippet instead. Unfortunately it does not preserve timestamps, but that is due to the bear python script being created from a cmake template file.
- In the future, please follow the pattern with URLs to the SPEC and SRPM on
the first two lines, as in the original post. The URLs should point to a raw SPEC and a directly downloadable SRPM. This allows a reviewer to use fedora-review.
Sorry, I forgot to do that.
- The file section can be simplified, e.g., instead of %dir %{_libdir}/bear/ %{_libdir}/bear/libear.so you can simply write %{_libdir}/bear Similarly for the docs.
Done
- You list some doc files twice. If you list a file such as "%doc
README.md", then you don't need to list it again. The build system already installs those files, so either you list them with %doc %{_docdir}/bear, or you remove them in %install and install them with %doc README.md etc. You also have two copies of COPYING. No need to have it in the doc dir, although that's not a big issue either.
Fixed that, the %files section became even shorter.
- Why do you have conditionals on %{?fedora}?
I usually build every package on CentOS/RHEL too and the tests were failing there, so I deactivated them on non-Fedora (and therefore also the buildrequires of python3-lit, which is only needed for the tests).
However, my recent build on CentOS failed, as rpmbuild isn't as clever on RHEL as it is on Fedora and it won't install the documentation properly.
[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Shebang_lines
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #26 from dan.cermak@cgc-instruments.com --- Spec URL: https://pagure.io/Bear_rpm/raw/master/f/bear.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/defolos/devel/fedora-29-x86_...
Just a minor fix for EPEL.
Are those %if 0%{?fedora} and %if 0%{?rhel} ok to keep in the package, although it is not being submitted for EPEL?
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #27 from Darryl T. Agostinelli dagostinelli@gmail.com --- A small comment.
- You can compact this:
%if 0%{?fedora} BuildRequires: python3-lit %endif
To this: %{?fedora:BuildRequires: python3-lit}
- Re: EPEL
it is not being submitted for EPEL
Of course that is your choice, though supporting EPEL can increase your package's reach. What's the issue?
https://bugzilla.redhat.com/show_bug.cgi?id=1565848
--- Comment #28 from dan.cermak@cgc-instruments.com --- (In reply to Darryl T. Agostinelli from comment #27)
it is not being submitted for EPEL
Of course that is your choice, though supporting EPEL can increase your package's reach. What's the issue?
Not a single issue but a combination of the following: - bear is my first package and I wanted to keep things simple - the tests are broken on EPEL and I have no clue how to fix them (haven't asked upstream yet though) - EPEL has very long support windows and I am very unsure whether I'd be able to backport changes
I therefore wanted to get the package into Fedora first and then maybe consider adding it to EPEL.
package-review@lists.fedoraproject.org