search

lopsided98 / nix-ros-overlay

ROS overlay for the Nix package manager
Apache License 2.0
192 stars 77 forks source link

Add fixupPhase flag to fix the cmake copy issue #397

Closed Thieso closed 2 weeks ago

Thieso commented 5 months ago

Fixes #391

But does this break anythink else? Is this a good fix?

wentasah commented 5 months ago

I think this should not break anything. Packages from this overlay are built with moveLib64 hook enabled so lib64 should always be a symlink to lib. buildEnv just creates one more level of symlinks, which should be fine.

The only downside I can see is that this approach may consume more i-nodes for symlinks than necessary. Creating one symlink from ...ros-env/lib64 to lib should be sufficient, but this creates full tree of new symlinks. But I don't think it's a problem for anybody.

hacker1024 commented 5 months ago

I'm a little concerned about potential performance impacts here. buildEnv is already quite slow, especially on devices with low-end storage.

Thieso commented 5 months ago

Fair, so would there be an easier way to achieve this? Maybe force overwriting of redundant files?

wentasah commented 5 months ago

I tried quick&dirty way of building all humble packages and only two of those, which build successfully, contain lib64. Namely foonathan-memory-vendor and google-benchmark-vendor. They together contain 23 entries in lib64, so the overhead created by buildEnv symlinks will be negligible.

Edit: With rolling, the situation is exactly the same.

Thieso commented 4 months ago

Would be great if this could be merged :) Thanks