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.
Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20240621gitce92fa7.fc39/basalt-2.0.1-1.20240621gitce92fa7.fc40.src.rpm Updated to latest and rebased patches. This is still very much a work in progress. The good news is upstream updated to the latest Pangolin so the likelihood we'll be able to add Pangolin as a separate package and then link against it has gone up. I have that ready to go and just waiting for an upstream release with the latest fixes. There is still alot of unbundling work to do and now tests are failing, likely as a result of the Pangolin update.
Created attachment 2037967 [details] The .spec file difference from Copr build 7445557 to 7648217
Copr build: https://copr.fedorainfracloud.org/coprs/build/7648217 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/07648217-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.
Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20240913git94ae6b6.fc40/basalt-2.0.1-1.20240913git94ae6b6.fc42.src.rpm [fedora-review-service-build]
Created attachment 2047041 [details] The .spec file difference from Copr build 7648217 to 8023382
Copr build: https://copr.fedorainfracloud.org/coprs/build/8023382 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/08023382-basalt/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20241104gitab5a3c7.fc40/basalt-2.0.1-1.20241104gitab5a3c7.fc41.src.rpm Still needs extra support to unbundle and get things in shape for inclusion. This is blocking monado.
Copr build: https://copr.fedorainfracloud.org/coprs/build/8361234 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/08361234-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.
> License: BSD-3-Clause This needs to also include all the licenses for the bundled libraries > # License: FIXME - unknown > Source10: %{forgeurl10}/archive/%{commit10}/ros_comm-%{commit10}.tar.gz > # License: FIXME - unknown > Source11: %{forgeurl11}/archive/%{commit11}/roscpp_core-%{commit11}.tar.gz This is a hard blocker, we can't include stuff with unknown licensing. > #BuildRequires: pkgconfig(benchmark) > #BuildRequires: pkgconfig(gtest) Add a comment to explain why these are commented out (or put them behind a bcond if that makes more sense). # Fedora pkgconfig(CLI11) = 2.4.2 #Provides: bundled(CLI11) = 2.0.0 Should this be removed? I see you do include the system one in the BRs earlier (likewise for the reference in %prep).
I'll start tracking down what to do with the ros* stuff.
Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20250321gitd55f57a.fc40/basalt-2.0.1-1.20250321gitd55f57a.fc43.src.rpm Changelog: - Update to latest commit There is still unbundling and license work needed.
Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20250404git4045633.fc40/basalt-2.0.1-1.20250404git4045633.fc43.src.rpm Changelog: - Update to latest commit - Rebase patch for GNUInstallDirs
Created attachment 2083550 [details] The .spec file difference from Copr build 8361234 to 8863888
Copr build: https://copr.fedorainfracloud.org/coprs/build/8863888 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/08863888-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.
https://gitlab.freedesktop.org/mateosss/basalt/-/issues/22 opened to sort out the licensing issues.
Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20250722gitdc6ef9f.fc41/basalt-2.0.1-1.20250722gitdc6ef9f.fc43.src.rpm Changelog: - Update to latest commit - Rebase patch for GNUInstallDirs
Created attachment 2103174 [details] The .spec file difference from Copr build 8863888 to 9395352
Copr build: https://copr.fedorainfracloud.org/coprs/build/9395352 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/09395352-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.
Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20250912git3388c6e.fc42.src.rpm Changelog: - Update to latest commit - Drop patch for GNUInstallDirs, merged upstream - Drop ROS requirement for library only builds, upstream cmake fixes (fixes last murky licensing issue)
Created attachment 2106544 [details] The .spec file difference from Copr build 9395352 to 9553563
Copr build: https://copr.fedorainfracloud.org/coprs/build/9553563 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/09553563-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.
This one has been left to wither on the vine so I will take it. Review to follow. Regards Phil
Hi, Partial review. * Build - Local: Fail ILD_SHARED_LIBRARY_ONLY=on -DBASALT_BUILD_TESTS=off -- The C compiler identification is GNU 16.0.1 -- The CXX compiler identification is GNU 16.0.1 -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Check for working C compiler: /usr/bin/gcc - skipped -- Detecting C compile features -- Detecting C compile features - done -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Check for working CXX compiler: /usr/bin/g++ - skipped -- Detecting CXX compile features -- Detecting CXX compile features - done -- Mold linker not found. Using default linker. -- Dind't find ccache -- Detected Linux with gcc. -- Eigen3 version .. found in /usr/include/eigen3, but at least version 3.4.0 is required CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:290 (message): Could NOT find Eigen3 (missing: EIGEN3_VERSION_OK) (Required is exact version "3.4.0") Call Stack (most recent call first): /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:654 (_FPHSA_FAILURE_MESSAGE) cmake_modules/FindEigen3.cmake:79 (find_package_handle_standard_args) CMakeLists.txt:271 (find_package) -- Configuring incomplete, errors occurred! error: Bad exit status from /var/tmp/rpm-tmp.2TgSnn (%build) Bad exit status from /var/tmp/rpm-tmp.2TgSnn (%build) RPM build errors: Finish: rpmbuild basalt-2.0.1-1.20250912git3388c6e.fc42.src.rpm Finish: build phase for basalt-2.0.1-1.20250912git3388c6e.fc42.src.rpm ERROR: Exception(SRPMS/basalt-2.0.1-1.20250912git3388c6e.fc42.src.rpm) Config(fedora-rawhide-x86_64) 5 minutes 25 seconds INFO: Results and/or logs in: /var/lib/mock/fedora-rawhide-x86_64/result ERROR: Command failed: * Build - COPR: Fail https://copr.fedorainfracloud.org/coprs/kathenas/generic-fedora-rawhide-all-with-review-not-production/build/10215835/ * Why are we importing third party packages rather than using Fedora system packages. I saw magic_enum first. Can we address these issues and then we can move forward getting the package into Fedora. Regards Phil
Thanks for picking this up! Now that someone is actually looking at it, I've put in some more unbundling effort. Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20260313git7e8ffb4.fc43/basalt-2.0.1-1.20260313git7e8ffb4.fc45.src.rpm Changelog: - Update to latest commit - Unbundle as much as I can get to build - Enable tests, for basalt only I have not done a deep dive on what patches need to go to what upstream yet. We might even just want to keep them downstream and report about them upstream. What do you think?
Created attachment 2133471 [details] The .spec file difference from Copr build 9553563 to 10226260
Copr build: https://copr.fedorainfracloud.org/coprs/build/10226260 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/10226260-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.
(In reply to Jonathan Steffan from comment #35) > Thanks for picking this up! Now that someone is actually looking at it, I've > put in some more unbundling effort. > > Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec > SRPM URL: > https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20260313git7e8ffb4. > fc43/basalt-2.0.1-1.20260313git7e8ffb4.fc45.src.rpm > > > Changelog: > > - Update to latest commit > - Unbundle as much as I can get to build > - Enable tests, for basalt only > > I have not done a deep dive on what patches need to go to what upstream yet. > We might even just want to keep them downstream and report about them > upstream. What do you think? Hi, I would do the deep dive and offer the patches to upstream and get acceptance or other feedback that can go in the packages spec file. It will help all in the long run. Line 21, can you add an empty line. It will separate the Source0,1,2,3 from other elements and make it easier to read. Bundled: # Best effort commit to version mapping for all thirdparty sources # No upstream versioning Provides: bundled(basalt-headers) # No upstream versioning Provides: bundled(opengv) Provides: bundled(sophus) = 1.22.10 Where possible can you provide upstream links for each of the above where necessary. This will make it easier for folks to jump to these projects for checking while they are not part of Fedora. If you choose to package any of these for Fedora, do let me know and I can pick up the reviews. I will do some further checking, but leave you with the above for now. Regards Phil
basalt-eigen3-version.patch - Updated upstream bug report. basalt-headers-system-cereal.patch - Could be submitted to basalt-headers. basalt-headers-system-eigen.patch - Could be submitted to basalt-headers. basalt-headers-system-gtest.patch - Could be submitted to basalt-headers. basalt-opengv-findEigen.patch - Needs to be submitted to opengv, which upstream has been silent for a while. Might be downstream only. basalt-system-cli11.patch - Downstream only, we are overriding the git submodule structure upstream is using. basalt-system-magic_enum.patch - Downstream only, we are overriding the git submodule structure upstream is using. Changelog: * Add documentation for bundles libraries * Add formatting changes Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-2.0.1-1.20260313git7e8ffb4.fc43/basalt-2.0.1-1.20260313git7e8ffb4.fc45.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/10226410 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/10226410-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.
Hi, Found some weekend time. One issue is the packages name. basalt[1] is a different actively. What is being packages is a fork of basalt with better XR devices with Monado[2]. The deb package referenced is 'basalt-monado' and I think we should follow this naming and leve it open for someone to package basalt proper at a later time if they so wish. Thouhghts? [1] https://gitlab.com/VladyslavUsenko/basalt [2] https://gitlab.freedesktop.org/mateosss/basalt/ Regards Phil
Phil, Great catch. Having hacked on this for so long I forgot it was a fork. Changelog: * Updated to the latest, including submodules for basalt-headers * Rebased patches * Renamed package to basalt-monado Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt-monado.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-monado-2.0.1-1.20260320git08dfe08.fc43/basalt-monado-2.0.1-1.20260320git08dfe08.fc45.src.rpm
Created attachment 2134554 [details] The .spec file difference from Copr build 10226410 to 10253095
Copr build: https://copr.fedorainfracloud.org/coprs/build/10253095 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2279244-basalt/fedora-rawhide-x86_64/10253095-basalt-monado/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.
Hi, Thanks for the package renaming. I will look at builds in this episode. Builds ====== * Local: x86_64: Good. * COPR: Fail on ppc64le architecture. - https://copr.fedorainfracloud.org/coprs/kathenas/generic-fedora-rawhide-all-with-review-not-production/build/10254969/ Partial of error: landmark_block.cpp cc1plus: error: unrecognized command-line option ‘-march=native’ ninja: build stopped: subcommand failed. error: Bad exit status from /var/tmp/rpm-tmp.8MgfaX (%build) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.8MgfaX (%build) Regards Phil
Hi, In this episode I am looking at the 'basalt-headers' primarily. The modified 'image.h' from Pangolin still falls under the MIT license? With also the 'Sophus' archive, should we have the 'License' element in the spec file be 'AND MIT'? License: BSD-3-Clause AND MIT Thoughts? Regards Phil
Changelog: * Updated to the latest * Update license * Exclusive arch for x86_64, aarch64 (I'm willing to debate fixing this, but in the rest of the XR packages we have just been dropping down to these two when there are issues) Spec URL: https://jsteffan.fedorapeople.org/imrsv/basalt-monado.spec SRPM URL: https://jsteffan.fedorapeople.org/imrsv/basalt-monado-2.0.1-1.20260325gitbec83db.fc43.src.rpm
Hi, Looking good here and many thanks for the hard work. One thing to maybe change in a future upload. The 'devel' package need not have '%license LICENSE' as the runtime package with the license is installed as a dep. I am marking the package as '+', so you can now request the repo for the package and import your initial srpm. Regards Phil
I've removed the duplicate LICENSE.
The Pagure repository was created at https://src.fedoraproject.org/rpms/basalt-monado
FEDORA-2026-0e7ebad6a1 (basalt-monado-2.0.1-1.20260325gitbec83db.fc45) has been submitted as an update to Fedora 45. https://bodhi.fedoraproject.org/updates/FEDORA-2026-0e7ebad6a1
FEDORA-2026-0e7ebad6a1 (basalt-monado-2.0.1-1.20260325gitbec83db.fc45) has been pushed to the Fedora 45 stable repository. If problem still persists, please make note of it in this bug report.