-
Notifications
You must be signed in to change notification settings - Fork 100
Allow generating vendored-source.json for packages with multiple ament_vendor calls #620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Allow generating vendored-source.json for packages with multiple ament_vendor calls #620
Conversation
2e602c0
to
512b21b
Compare
…t_vendor calls This is needed at least for zenoh-cpp-vendor, which is being prepared in lopsided98#558. The format of vendored-source.json is changed from a single object to an array of objects. The order of objects corresponds to the order of ament_vendor calls in the patched CMakeLists.txt. ament_cmake_vendor_packageConfig.cmake was updated to generate such a format automatically and patchAmentVendorGit was updated to understand that format and patch all call sites in the correct order. The vendored-source.json files are regenerated automatically and there are no changes except addition of extra brackets.
This is required since nix-eval-jobs version 2.26.0. Without this, we don't find any package to update.
This makes not change for currently vendored packages, but it will be needed for zenoh-cpp-vendor.
512b21b
to
2913d26
Compare
Fixed patching to build |
Even this is not good. This version fails with |
…rences The number of entries in vendored-source.json matches the actual number of calls made during cmake execution. But the CMakeLists.txt can contain multiple calls guarded with different if conditions, as is the case with rviz-ogre-vendor. To be sure, that we patch the correct call, we needed to patch all of them. On the other hand, if the actual number of calls is greater than one (as with zenoh-cpp-vendor), we patch them one-by-one, hoping that there are no calls guarded by false if conditions. This heuristic works for all packages that are currently patched with patchAmentVendorGit.
Now it works for all packages in the overlay as well as for |
Thanks for your work. Now everything works fine. As soon as this gets merged I will update #558 . |
'' | ||
sed -i '/VCS_TYPE \(git\|zip\|svn\|path\)/d' ${lib.escapeShellArg file} | ||
'' + ( | ||
if length sourceInfos == 1 then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a bit brittle. Not that big a deal since we only have to handle a limited number of examples, but as an alternative, would it be feasible to match against the original VCS_URL
value when patching?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately not. CMakeFile authors are very creative and use variables in URLs like:
VCS_URL https://github.com/gazebosim/${GITHUB_NAME}.git
I was also looking at whether cmake
can tell us the line number from where a macro (of a function) was called so that we can match based on that, but nothing like this seems to exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it could be done if there were some way to wrap the ament_vendor()
call with our own macro that replaces the VCS_URL
parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem to solve here would be how to call the original ament_vendor
implementation from the wrapper with the same name. I don't see any means for macro namespacing in cmake. Maybe, we can use Nix to patch the original implementation to have a different name and call that from the wrapper.
@wentasah I have updated my test branch for this PR to be up to date with the develop branch. error: builder for '/nix/store/3qd6hzm9f9lj028bqcvhi2xyrc0kndgc-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1.drv' failed with exit code 2;
last 25 log lines:
> CMAKE_INSTALL_LOCALEDIR
> CMAKE_INSTALL_MANDIR
> CMAKE_INSTALL_NAME_DIR
> CMAKE_INSTALL_SBINDIR
> CMAKE_POLICY_DEFAULT_CMP0025
>
>
> -- Build files have been written to: /build/rmw_zenoh-release-release-jazzy-zenoh_cpp_vendor-0.2.4-1/build
> cmake: enabled parallel building
> cmake: enabled parallel installing
> Running phase: buildPhase
> build flags: -j64 SHELL=/nix/store/58br4vk3q5akf4g8lx0pqzfhn47k3j8d-bash-5.2p37/bin/bash
> [ 6%] Creating directories for 'zenoh_c_vendor'
> [ 12%] Performing download step for 'zenoh_c_vendor'
> === ./zenoh_c_vendor (tar) ===
> Downloaded tarball from 'file:///nix/store/5jnxif96jdmdqg0fnqjgjy023k2bcl7p-e6a1971139f405f7887bf5bb54f0efe402123032.tar' and unpacked it
> [ 18%] No update_disconnected step for 'zenoh_c_vendor'
> [ 25%] No patch_disconnected step for 'zenoh_c_vendor'
> [ 31%] Performing configure step for 'zenoh_c_vendor'
> loading initial cache file /build/rmw_zenoh-release-release-jazzy-zenoh_cpp_vendor-0.2.4-1/build/zenoh_c_vendor-config.cmake
> CMake Error: The source directory "/build/rmw_zenoh-release-release-jazzy-zenoh_cpp_vendor-0.2.4-1/build/zenoh_c_vendor-prefix/src/zenoh_c_vendor" does not appear to contain CMakeLists.txt.
> Specify --help for usage, or press the help button on the CMake GUI.
> make[2]: *** [CMakeFiles/zenoh_c_vendor.dir/build.make:93: zenoh_c_vendor-prefix/src/zenoh_c_vendor-stamp/zenoh_c_vendor-configure] Error 1
> make[1]: *** [CMakeFiles/Makefile2:156: CMakeFiles/zenoh_c_vendor.dir/all] Error 2
> make: *** [Makefile:146: all] Error 2
For full logs, run 'nix log /nix/store/3qd6hzm9f9lj028bqcvhi2xyrc0kndgc-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1.drv'. Also updating of the ============== Updating ros-jazzy-zenoh-cpp-vendor ==============
Running phase: unpackPhase
unpacking source archive /nix/store/0wl79ahilcmmvr45gfgcpsa9lhhn9pcq-0.2.4-1.tar.gz
source root is rmw_zenoh-release-release-jazzy-zenoh_cpp_vendor-0.2.4-1
setting SOURCE_DATE_EPOCH to timestamp 1745217032 of file "rmw_zenoh-release-release-jazzy-zenoh_cpp_vendor-0.2.4-1/package.xml"
Running phase: patchPhase
Running phase: updateAutotoolsGnuConfigScriptsPhase
Running phase: configurePhase
fixing cmake files...
cmake flags: -DCMAKE_FIND_USE_SYSTEM_PACKAGE_REGISTRY=OFF -DCMAKE_FIND_USE_PACKAGE_REGISTRY=OFF -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=OFF -DCMAKE_INSTALL_LOCALEDIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/share/locale -DCMAKE_INSTALL_LIBEXECDIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/libexec -DCMAKE_INSTALL_LIBDIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/lib -DCMAKE_INSTALL_DOCDIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/share/doc/zenoh_cpp_vendor -DCMAKE_INSTALL_INFODIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/share/info -DCMAKE_INSTALL_MANDIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/share/man -DCMAKE_INSTALL_INCLUDEDIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/include -DCMAKE_INSTALL_SBINDIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/sbin -DCMAKE_INSTALL_BINDIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/bin -DCMAKE_INSTALL_NAME_DIR=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source/lib -DCMAKE_POLICY_DEFAULT_CMP0025=NEW -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_STRIP=/nix/store/8v6k283dpbc0qkdq81nb6mrxrgcb10i1-gcc-wrapper-14-20241116/bin/strip -DCMAKE_RANLIB=/nix/store/8v6k283dpbc0qkdq81nb6mrxrgcb10i1-gcc-wrapper-14-20241116/bin/ranlib -DCMAKE_AR=/nix/store/8v6k283dpbc0qkdq81nb6mrxrgcb10i1-gcc-wrapper-14-20241116/bin/ar -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_INSTALL_PREFIX=/nix/store/mp61izang6dii67jlbfhjrifgbqg7y2c-ros-jazzy-zenoh-cpp-vendor-0.2.4-r1-source -DAMENT_CMAKE_ENVIRONMENT_PARENT_PREFIX_PATH_GENERATION:BOOL=OFF -DPython3_EXECUTABLE:FILEPATH=/nix/store/f2krmq3iv5nibcvn4rw7nrnrciqprdkh-python3-3.12.9/bin/python3.12 -DAMENT_CMAKE_ENVIRONMENT_PARENT_PREFIX_PATH_GENERATION:BOOL=OFF -DPython3_EXECUTABLE:FILEPATH=/nix/store/f2krmq3iv5nibcvn4rw7nrnrciqprdkh-python3-3.12.9/bin/python3.12
-- The C compiler identification is GNU 14.2.1
-- The CXX compiler identification is GNU 14.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /nix/store/8v6k283dpbc0qkdq81nb6mrxrgcb10i1-gcc-wrapper-14-20241116/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: /nix/store/8v6k283dpbc0qkdq81nb6mrxrgcb10i1-gcc-wrapper-14-20241116/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found ament_cmake: 2.5.4 (/nix/store/1bcaqlmsj8h66v9bqri12iwaha1b7n2s-ros-jazzy-ament-cmake-2.5.4-r1/share/ament_cmake/cmake)
-- Found Python3: /nix/store/f2krmq3iv5nibcvn4rw7nrnrciqprdkh-python3-3.12.9/bin/python3.12 (found version "3.12.9") found components: Interpreter
-- Found ament_cmake_vendor_package: Dummy version for Nix patching (/nix/store/dyclrz7njjz961z11sdp73x92bqhlqvf-ament_cmake_vendor_package_nix)
'/nix/store/gg1b5asslgwzmh0cznwxvpclwpg2706f-nix-prefetch-git/bin/nix-prefetch-git' '--url' 'https://github.com/eclipse-zenoh/zenoh-c.git' '--rev' 'ffa4bddc947f7ed6c0e3b4546205dd1b73e7df81' '--fetch-submodules'
'/nix/store/gpxsdrrd4x93fs75395vr2dfys1ki9mq-jq-1.7.1-bin/bin/jq' '[] + [{url: "https://github.com/eclipse-zenoh/zenoh-c.git", rev: "ffa4bddc947f7ed6c0e3b4546205dd1b73e7df81", hash: .hash}]'
Initialized empty Git repository in /tmp/nix-shell-260601-0/git-checkout-tmp-oaAQjBEX/zenoh-c-ffa4bdd/.git/
From https://github.com/eclipse-zenoh/zenoh-c
* branch ffa4bddc947f7ed6c0e3b4546205dd1b73e7df81 -> FETCH_HEAD
Switched to a new branch 'fetchgit'
removing `.git'...
/nix/store/gg1b5asslgwzmh0cznwxvpclwpg2706f-nix-prefetch-git/bin/.nix-prefetch-git-wrapped: line 468: nix-hash: command not found
CMake Error at /nix/store/dyclrz7njjz961z11sdp73x92bqhlqvf-ament_cmake_vendor_package_nix/ament_cmake_vendor_packageConfig.cmake:34 (execute_process):
execute_process failed command indexes:
1: "Child return code: 127"
Call Stack (most recent call first):
CMakeLists.txt:37 (ament_vendor)
-- Configuring incomplete, errors occurred! Can you try to reproduce that? Edit: |
I'm reworking this based on above discussion with @lopsided98, but it takes me longer than expected due to other duties. I'd like to have it ready this week. I'll then look at zenoh-cpp-vendor because without it, we won't beady for Kilted release. |
This is needed at least for
zenoh-cpp-vendor
, which is being prepared in #558.The format of
vendored-source.json
is changed from a single object to an array of objects. The order of objects corresponds to the order ofament_vendor
calls in the patchedCMakeLists.txt
.ament_cmake_vendor_packageConfig.cmake
was updated to generate such a format automatically andpatchAmentVendorGit
was updated to understand that format and patch all call sites in the correct order.The
vendored-source.json
files are regenerated automatically and there are no changes except addition of extra brackets.