clearpathrobotics / nix-ros-base

43 stars 3 forks source link

Fail to build rolling base #4

Closed haudren-woven closed 2 years ago

haudren-woven commented 2 years ago

Thanks of all, thanks for putting this repository up, it's very helpful to experiment with Nix! I wanted to try to build a ROS2 workspace, and so I typed:

nix build ros/20221020-1#rolling.ros_base.ws.contents

Which fails with:

error: builder for '/nix/store/ssszwq15pba7d9m3phaw3snpy1p2r134-Mimick-a819614.drv' failed with exit code 1;
       last 10 log lines:
       >  * [new branch]      features/travis       -> origin/features/travis
       >  * [new branch]      features/windows      -> origin/features/windows
       >  * [new branch]      master                -> origin/master
       >  * [new branch]      ros2                  -> origin/ros2
       >  * [new tag]         v0.1.0                -> v0.1.0
       >  * [new tag]         v0.2.0                -> v0.2.0
       >  * [new tag]         v0.3.0                -> v0.3.0
       > fatal: git cat-file: could not get object info
       > Unrecognized git object type:
       > Unable to checkout a819614f43592f551697e5bc9dba8a16e7d9f44d from https://github.com/ros2/Mimick.git.
       For full logs, run 'nix log /nix/store/ssszwq15pba7d9m3phaw3snpy1p2r134-Mimick-a819614.drv'.
error: 1 dependencies of derivation '/nix/store/lyj6g003x07nhmxcs0878dzvszdfcmny-mimick_vendor.drv' failed to build

It seems like the flake for mimick is attempting to download the wrong git tag. The data in the nix flake seems to point to the correct tag for mimick_vendor ( https://github.com/clearpathrobotics/nix-ros/blob/latest/rolling/srcs/mimick_vendor.nix ). However, mimick_vendor adds mimick as an external project using CMake. Using nix show-derivation ros#rolling.mimick_vendor I see that there is an override applied to that package, that uses this store path as the git repository:

/nix/store/ssszwq15pba7d9m3phaw3snpy1p2r134-Mimick-a819614.drv

      "postPatch": "sed -i '\\|GIT_REPOSITORY\\s.*'https://github.com/ros2/Mimick.git'|c\\\n  URL \"/nix/store/90lqhsfb4ss99a3g67k3g8x8iqih1lq4-Mimick-a819614\"' \\\n  'CMakeLists.txt'\n",

However, the CMakeLists of mimick_vendor specifies that we should download Mimick at version de11f8377eb95f932a03707b583bf3d4ce5bd3e7. I have been looking around, but since I'm really new with Nix, I can't quite quite figure out just yet how that a819614 reference got into the system, although it seems like the rolling override is to blame :thinking:

Thanks again for the great talk at ROSCon!

haudren-woven commented 2 years ago

Actually this little patch fixes the wrong git tag, I didn't know the overrides were manually specified:

diff --git a/overrides/rolling.nix b/overrides/rolling.nix
index fe42b41..fc7de4a 100644
--- a/overrides/rolling.nix
+++ b/overrides/rolling.nix
@@ -92,8 +92,8 @@ in rosFinal: rosPrev: {
   mimick_vendor = (patchVendorGit rosPrev.mimick_vendor {
     url = "https://github.com/ros2/Mimick.git";
     fetchgitArgs = {
-      rev = "a819614f43592f551697e5bc9dba8a16e7d9f44d";
-      sha256 = "sha256-LHz0xFt7Q1HKPJBDUnQoHEPbWLZ2zBwj4vxy0LZ2c5c=";
+      rev = "de11f8377eb95f932a03707b583bf3d4ce5bd3e7";
+      sha256 = "sha256-adCxIl0F3QkgSimOhvuTyhmig1rFy/K9wxZ/+YCuxYo=";
     };
   });

However, I am now getting another error....

/nix/store/hyz46mc9bnjl6rq6hzbwjaybjybzav8c-glibc-2.35-163-dev/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [8;;https://gcc.gnu.org/onlinedocs/gcc/>
  412 | #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)

But the flags.txt seems to correctly specify -O2 so I need to figure out why mimick does not see those flags...

haudren-woven commented 2 years ago

And for completeness' sake, I found out what is the problem:

I don't know why the compiler warns against using FORTIFY_SOURCE without optimization, but that's a very unfortunate choice...

mikepurvis commented 2 years ago

Hi @haudren-woven, thanks for giving the repo a try and for raising this bug! I suspect we didn't catch this when preparing this demo as we already had the source in Mimick source in our nix stores, so it didn't have to rebuild.

I will look at how to correct this so that it's properly rebuildable from source.

Overall, it would be great to find a more generic solution for the ROS 2 vendor packages than having to manually maintain these override patches— one option could be another plugin similar to colcon-nix that could seek out this pattern and save the necessary information in the distro cache so that we could simply generate the substitutions.

haudren-woven commented 2 years ago

Overall, it would be great to find a more generic solution for the ROS 2 vendor packages than having to manually maintain these override patches— one option could be another plugin similar to colcon-nix that could seek out this pattern and save the necessary information in the distro cache so that we could simply generate the substitutions.

That would be a great improvement! I just wish there was no need for vendored packages as it always seems pretty hacky to do textual replacement on the CMake sources :(

mikepurvis commented 2 years ago

It's definitely a pivot from how the ROS 1 philosophy, which strongly preferred using upstream packages, and pushing things upstream. But that cost of that is that porting to new platforms is very expensive— ROS 2 is much more self contained and widely available as a consequence of this change in direction.

Another possible way to deal with this in Nix specifically would be to just completely override the _vendor packages and replace them with the upstream versions available in nixpkgs. I experimented with this approach and ran into some minor trouble with name mapping and patch application. Would be delighted for a community member to pick up that work and find a better way forward, though.