Bug 2432762

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10065211
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2432762-cpptrace/fedora-rawhide-x86_64/10065211-cpptrace/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.

Spec URL: https://download.copr.fedorainfracloud.org/results/davidsch/testa/fedora-rawhide-aarch64/10359801-cpptrace/cpptrace.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/davidsch/testa/fedora-rawhide-aarch64/10359801-cpptrace/cpptrace-1.0.4-1.fc45.src.rpm

I have fixed the URL, thanks @qulogic for noticing!

Created attachment 2138038 [details]
The .spec file difference from Copr build 10065211 to 10359809

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10359809
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2432762-cpptrace/fedora-rawhide-x86_64/10359809-cpptrace/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.

I will finish looking at when I get to the office.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed

===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Package contains no static executables.
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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: "Unknown or generated", "MIT License". 207 files have unknown
     license. Detailed output of licensecheck in
     JL: It seems that many of the project files don't explicitly mention a license that
         doesn't change the overall project license which notes it is licensed as LGPL if
	 statically linked with libdwarf, but that isn't the case with the current build.
[X}: License file installed when any subpackage combination is installed.
[X]: %build honors applicable compiler flags or justifies otherwise.
     JL: Looks that way, final build on aarch64 includes PAC,BTI,GCS indicating
     the -mbranch-protection flags are working their way through the build correctly.
[X]: Package contains no bundled libraries or specifies bundled libraries
     with Provides: bundled(<libname>) if unbundling is not possible.
[X]: Changelog in prescribed format.
     JL: Changelog includes some bogus email/commit and should be cleaned up before final
         check-in.
[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.
     JL: The use of rpmautospec/etc has created a few comment/style issues that
         while I don't believe are wrong could use some minor tweaking before final.
[-]: 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.
[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.
[!]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages which are apparently fine
     But I think its complaining about the libcpptrace.so.1 which shouldn't be in the -devel package,
     and the associated missing ldconfig/symlink error goes with it.
[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 must own all directories that it creates.
[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 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]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[X]: 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).
[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     cpptrace-devel
[X]: Package functions as described.
[X]: Latest version is packaged.
[X]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
     JL: I don't see an upstream signature, so I'm not going to fail this, but i personally tend to
     believe in these cases that using the release git SHA and downloading it that way: ex:
     https://github.com/jeremy-rifkin/cpptrace/archive/%{cpptrace_tag}.tar.gz
     tends to kill a few of the obvious version shenanigans (knowing of course that the look-aside also
     solves this to a certain extent)
[X]: Package should compile and build into binary rpms on all supported
     architectures.
[!]: %check is present and all tests pass.
     JL: Gonna fail this one, because gtest is already packaged and there are packages
         that depend on it, maybe a quick tweak allows the tests to run?
[X]: Packages should try to preserve timestamps of original installed
     files.
[!]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define autorelease(e:s:pb:n)
     %{?-p:0.}%{lua:
     I guess i don't know what this is accomplishing/aka why its needed here.
[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.

===== 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: cpptrace-1.0.4-1.fc45.aarch64.rpm
          cpptrace-devel-1.0.4-1.fc45.aarch64.rpm
          cpptrace-1.0.4-1.fc45.src.rpm
============================ rpmlint session starts ============================
rpmlint: 2.8.0
configuration:
    /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpvgj2qst1')]
checks: 32, packages: 3

cpptrace.aarch64: E: spelling-error ('stacktrace', 'Summary(en_US) stacktrace -> stack trace, stack-trace, racetrack')
cpptrace.aarch64: E: spelling-error ('stacktrace', '%description -l en_US stacktrace -> stack trace, stack-trace, racetrack')
cpptrace.aarch64: E: spelling-error ('macOS', '%description -l en_US macOS -> ma Cos, mac OS, mac-OS')
cpptrace.aarch64: E: spelling-error ('stacktraces', '%description -l en_US stacktraces -> stack traces, stack-traces, racetracks')
cpptrace.aarch64: E: spelling-error ('triaging', '%description -l en_US triaging -> trialing, stringing, training')
cpptrace.src: E: spelling-error ('stacktrace', 'Summary(en_US) stacktrace -> stack trace, stack-trace, racetrack')
cpptrace.src: E: spelling-error ('stacktrace', '%description -l en_US stacktrace -> stack trace, stack-trace, racetrack')
cpptrace.src: E: spelling-error ('macOS', '%description -l en_US macOS -> ma Cos, mac OS, mac-OS')
cpptrace.src: E: spelling-error ('stacktraces', '%description -l en_US stacktraces -> stack traces, stack-traces, racetracks')
cpptrace.src: E: spelling-error ('triaging', '%description -l en_US triaging -> trialing, stringing, training')
cpptrace.aarch64: E: no-ldconfig-symlink /usr/lib64/libcpptrace.so.1.0.4
cpptrace.aarch64: W: no-documentation
cpptrace-devel.aarch64: W: no-documentation
cpptrace.aarch64: W: name-repeated-in-summary Cpptrace
cpptrace.src: W: name-repeated-in-summary Cpptrace
cpptrace-devel.aarch64: W: dangling-relative-symlink /usr/lib64/libcpptrace.so.1 libcpptrace.so.1.0.4
 3 packages and 0 specfiles checked; 11 errors, 5 warnings, 19 filtered, 11 badness; has taken 0.7 s 




Rpmlint (debuginfo)
-------------------
Checking: cpptrace-debuginfo-1.0.4-1.fc45.aarch64.rpm
============================ rpmlint session starts ============================
rpmlint: 2.8.0
configuration:
    /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
rpmlintrc: [PosixPath('/tmp/tmpx1b12s3r')]
checks: 32, packages: 1

 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 5 filtered, 0 badness; has taken 0.4 s 





Rpmlint (installed packages)
----------------------------
============================ rpmlint session starts ============================
rpmlint: 2.9.0
configuration:
    /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 32, packages: 3

cpptrace.aarch64: E: spelling-error ('stacktrace', 'Summary(en_US) stacktrace -> stack trace, stack-trace, backtrace')
cpptrace.aarch64: E: spelling-error ('stacktrace', '%description -l en_US stacktrace -> stack trace, stack-trace, backtrace')
cpptrace.aarch64: E: spelling-error ('stacktraces', '%description -l en_US stacktraces -> stack traces, stack-traces, backtraces')
cpptrace.aarch64: E: no-ldconfig-symlink /usr/lib64/libcpptrace.so.1.0.4
cpptrace.aarch64: W: no-documentation
cpptrace-devel.aarch64: W: no-documentation
cpptrace.aarch64: W: name-repeated-in-summary Cpptrace
cpptrace-devel.aarch64: W: dangling-relative-symlink /usr/lib64/libcpptrace.so.1 libcpptrace.so.1.0.4
 3 packages and 0 specfiles checked; 4 errors, 4 warnings, 21 filtered, 4 badness; has taken 0.5 s 



Source checksums
----------------
https://github.com/jeremy-rifkin/cpptrace/archive/refs/tags/v1.0.4.tar.gz :
  CHECKSUM(SHA256) this package     : 5c9f5b301e903714a4d01f1057b9543fa540f7bfcc5e3f8bd1748e652e24f9ea
  CHECKSUM(SHA256) upstream package : 5c9f5b301e903714a4d01f1057b9543fa540f7bfcc5e3f8bd1748e652e24f9ea


Requires
--------
cpptrace (rpmlib, GLIBC filtered):
    ld-linux-aarch64.so.1()(64bit)
    libc.so.6()(64bit)
    libdwarf.so.2()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.3)(64bit)
    libgcc_s.so.1(GCC_3.3.1)(64bit)
    libgcc_s.so.1(GCC_4.2.0)(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.13)(64bit)
    libstdc++.so.6(CXXABI_1.3.15)(64bit)
    libstdc++.so.6(CXXABI_1.3.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.5)(64bit)
    libstdc++.so.6(CXXABI_1.3.7)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    rtld(GNU_HASH)

cpptrace-devel (rpmlib, GLIBC filtered):
    cmake-filesystem(aarch-64)
    libcpptrace.so.1()(64bit)
    libzstd-devel



Provides
--------
cpptrace:
    cpptrace
    cpptrace(aarch-64)
    libcpptrace.so.1()(64bit)

cpptrace-devel:
    cmake(cpptrace)
    cpptrace-devel
    cpptrace-devel(aarch-64)



Generated by fedora-review 0.11.0 (05c5b26) last change: 2025-11-29
Command line :/usr/bin/fedora-review -b 2432762
Buildroot used: fedora-rawhide-aarch64
Active plugins: Generic, C/C++, Shell-api
Disabled plugins: Python, Ocaml, R, fonts, SugarActivity, Haskell, Perl, PHP, Java
Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH

I failed a couple minor things, which in a couple cases could be my own understanding... The notes usually preceded by "JL:" under each check includes various comments of mine. The ones without are largely fedora-review itself.

     JL: It seems that many of the project files don't explicitly mention a license that
         doesn't change the overall project license which notes it is licensed as LGPL if
	 statically linked with libdwarf, but that isn't the case with the current build.

The Readme mentiones explicitly that it is MIT.

It is a bit confusing that they spend more text talking about the implications of linking, than about their license.


     JL: Changelog includes some bogus email/commit and should be cleaned up before final
         check-in.

I do not intend to import the git history, so that should be fine, once it is imported.


     JL: The use of rpmautospec/etc has created a few comment/style issues that
         while I don't believe are wrong could use some minor tweaking before final.

Just at the beginning and end? That will be resolved once it is imported.
If there is something else in the main part of the spec, I am happy for comments, so I can improve.


     JL: I don't see an upstream signature, so I'm not going to fail this, but i personally tend to
     believe in these cases that using the release git SHA and downloading it that way: ex:
     https://github.com/jeremy-rifkin/cpptrace/archive/%{cpptrace_tag}.tar.gz
     tends to kill a few of the obvious version shenanigans (knowing of course that the look-aside also
     solves this to a certain extent)

You mean with %{cpptrace_tag} the sha1 hash of the upsteam commit, that got tagged?
If so, I think this is not particularly helpful. Sha1 is not super secure, and that only helps for tempering with the release, after it is released.
At which point it is already in the lookaside cache. So we would not see the modified files.
In addition, we store the sha512 signature, any changes here would fail the build anyway.
To me that seems like a lot of extra work, with no benefit. If at all, it makes it for a casual reviewer harder to spot a mismatch between the official release and the commit sha.
I have not checked this also true for the `archive` API endpoint, but if you reference a commit from a fork in the main repo, you can fool people into thinking that is part of the official repo, while it is not. This would allow someone to send a PR to "update to the newest upstream release", but instead of the official hash, reference a bogus commit from there fork, that includes whatever. With sha1, it should even be possible to match the parts of the beginning or end, to make even a manual check of that hash potentially pass.

     JL: Gonna fail this one, because gtest is already packaged and there are packages
         that depend on it, maybe a quick tweak allows the tests to run?

Indeed, thanks. I have added the unit tests.

Here are the updated files:
Spec URL: https://download.copr.fedorainfracloud.org/results/davidsch/testa/fedora-rawhide-aarch64/10404971-cpptrace/cpptrace.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/davidsch/testa/fedora-rawhide-aarch64/10404971-cpptrace/cpptrace-1.0.4-1.fc45.src.rpm

Created attachment 2138554 [details]
The .spec file difference from Copr build 10359809 to 10405287

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10405287
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2432762-cpptrace/fedora-rawhide-x86_64/10405287-cpptrace/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.

Right, some of the JL: comments were on areas I was making notes for posterity (aka the license ones), and were "X" passed.

But: there is this:
[!]: Rpmlint is run on all rpms the build produces.
     JL: There are rpmlint messages which are apparently fine
     But I think its complaining about the libcpptrace.so.1 which shouldn't be in the -devel package,
     and the associated missing ldconfig/symlink error goes with it.

Which I should have JL'ed, because I still see that it looks like %{_libdir}/libcpptrace.so.1 needs to be moved from -devel to the main package. That is what is causing some of the rpmlint issues. The fedora guidelines state (AFAIK/IMHO) its only the unversioned DSO that must be in the -devel. 

And AFAIK should suppress the rpmlint error:
cpptrace.aarch64: E: no-ldconfig-symlink /usr/lib64/libcpptrace.so.1.0.4

Also, the previous comment:
     Note: %define requiring justification: %define autorelease(e:s:pb:n)
     %{?-p:0.}%{lua:
     JL: I guess i don't know what this is accomplishing/aka why its needed here.

which AFAIK shouldn't be overridden since its been in place for a while now in fedora, is this old RHEL drippings? AKA I think you can just drop it rather than %define'ing the global.

Other than that I think it looks good.

(In reply to Jeremy Linton from comment #11)
> Right, some of the JL: comments were on areas I was making notes for
> posterity (aka the license ones), and were "X" passed.
> 
> But: there is this:
> [!]: Rpmlint is run on all rpms the build produces.
>      JL: There are rpmlint messages which are apparently fine
>      But I think its complaining about the libcpptrace.so.1 which shouldn't
> be in the -devel package,
>      and the associated missing ldconfig/symlink error goes with it.
> 
> Which I should have JL'ed, because I still see that it looks like
> %{_libdir}/libcpptrace.so.1 needs to be moved from -devel to the main
> package. That is what is causing some of the rpmlint issues. The fedora
> guidelines state (AFAIK/IMHO) its only the unversioned DSO that must be in
> the -devel. 
> 
> And AFAIK should suppress the rpmlint error:
> cpptrace.aarch64: E: no-ldconfig-symlink /usr/lib64/libcpptrace.so.1.0.4

I moved the so.1 to the main package. I think it also should be there, due to the Provides.

I also added the Requires: cpptrace%{?_isa} = %{version}-%{release}
to the devel package.

> Also, the previous comment:
>      Note: %define requiring justification: %define autorelease(e:s:pb:n)
>      %{?-p:0.}%{lua:
>      JL: I guess i don't know what this is accomplishing/aka why its needed
> here.
> 
> which AFAIK shouldn't be overridden since its been in place for a while now
> in fedora, is this old RHEL drippings? AKA I think you can just drop it
> rather than %define'ing the global.
> 
> Other than that I think it looks good.

That is from autorelease. I do not add that, see the unprocessed spec, that will be imported:
https://raw.githubusercontent.com/dschwoerer/cpptrace-review/refs/heads/main/cpptrace.spec

Sorry for not mentioning that earlier.

Updated links:
Spec URL: https://raw.githubusercontent.com/dschwoerer/cpptrace-review/refs/heads/main/cpptrace.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/davidsch/testa/fedora-rawhide-aarch64/10421647-cpptrace/cpptrace-1.0.4-1.fc45.src.rpm

Created attachment 2139366 [details]
The .spec file difference from Copr build 10405287 to 10421914

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10421914
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2432762-cpptrace/fedora-rawhide-x86_64/10421914-cpptrace/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.

Looks good, thanks for taking the time to work on this.