Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-0-1.20240310git673cc5c.fc39/basalt-0-1.20240310git673cc5c.fc39.src.rpm Description: Visual-Inertial Mapping with Non-Linear Factor Recovery Fedora Account System Username: jsteffan
Copr build: https://copr.fedorainfracloud.org/coprs/build/7418841 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/07418841-basalt/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.
Some notes from a quick skim: > tar -xvf %{SOURCE1} --strip-components 1 -C thirdparty/CLI11 > tar -xvf %{SOURCE4} --strip-components 1 -C thirdparty/magic_enum These two are header-only libraries and they're already packaged, you might be able to add them as BRs and symlink them in place to unbundle them > tar -xvf %{SOURCE8} --strip-components 1 -C thirdparty/basalt-headers/thirdparty/eigen You also have eigen3 in the BRs. Is this really using _both_ the system eigen3 and the bundled eigen? > # CMake errors: > # -DBUILD_TESTS=on \ > #thirdparty/basalt-headers/test/googletest does not contain a CMakeLists.txt file > #thirdparty/basalt-headers/test/benchmark does not contain a CMakeLists.txt file This is because basalt-headers pulls these two in via submodules: https://gitlab.freedesktop.org/mateosss/basalt-headers/-/tree/main/test > %{_libdir}/libbasalt.so.* You want to glob the soversion here so we don't accidentally bump it without noticing > %{_datarootdir}/basalt/* You should own the whole directory, not just its contents
Thanks for the feedback with this still in such a raw state. Cleanup of the spec will happen after all outstanding issues are solved. I was able to use the system CLI11 and unbundle that. The Fedora magic_enum package installs into /usr/include so a simple symlink wont work. Eigen3 is more stubborn. We should be able to use the system eigen3, and it's required to be installed for checks to pass for the main source code, but getting the bundled basalt-headers to do the right thing has resulted in the likely need to patch thirdparty/basalt-headers/CMakeLists.txt. I wasn't able to find a configuration that would use the system libs and still pass their custom cmake function logic. Details in the updated spec. Tests fixed and are running now, thanks for point out those additional git submodules. Hopefully we've found them all now. %files updated based on feedback. Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-0-1.20240310git673cc5c.fc39/basalt-0-1.20240310git673cc5c.fc39.src.rpm
Created attachment 2032812 [details] The .spec file difference from Copr build 7418841 to 7436955
Copr build: https://copr.fedorainfracloud.org/coprs/build/7436955 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/07436955-basalt/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.
Added missing bundled for benchmark and googletest. Those might be able to be unbundled. Notes left in the spec. Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-0-1.20240310git673cc5c.fc39/basalt-0-1.20240310git673cc5c.fc39.src.rpm
Created attachment 2032813 [details] The .spec file difference from Copr build 7436955 to 7436968
Copr build: https://copr.fedorainfracloud.org/coprs/build/7436968 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/07436968-basalt/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.
Additional work on trying to unbundle. Updated spec with more notes and FIXMEs. Updated to the latest, which included a soname bump. Changed the version to the soname version and we will rely on %autorelease for updates without soname bumps. (if that's fine.) Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20240513gitc03cbb3.fc39/basalt-2.0.1-1.20240513gitc03cbb3.fc39.src.rpm
Created attachment 2033284 [details] The .spec file difference from Copr build 7436968 to 7445557
Copr build: https://copr.fedorainfracloud.org/coprs/build/7445557 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/07445557-basalt/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.