Skip to content
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

Adding all dependencies to propagatedBuildInputs is not sustainable. #544

Open
liarokapisv opened this issue Dec 13, 2024 · 3 comments
Open

Comments

@liarokapisv
Copy link
Contributor

liarokapisv commented Dec 13, 2024

The heavy usage of propagatedBuildInputs means that projects with many dependencies end up collecting a ton of gcc flags through the cc-wrapper.

This includes - for each dependency in the dependency chain - the include folder path and the lib folder path together.
Each path is ~80 characters on average.

When using a dev-shell for a workspace, it will pickup a ton of transitive dependencies from all the workspace packages.

As an example, my devshell environment ends up with ~260 dependencies (because they are transitive).

For some reason these get a x3 duplication in the cc-wrapper invocation, probably due to a cc-wrapper setup-hook.sh omition when propagatedBuildInputs or something, I am still investigating the reason, it seems x2 duplication is a nixpkgs issue NixOS/nixpkgs#364984 not sure where the extra duplication comes from.

This means that any gcc invocation ends up with x3 x260 x2 x80 = 124800 kb worth of flags.

Add all the extra -system and additional gcc crafts and we end up hitting NixOS/nixpkgs#41340

This was not an issue for me with an older master commit, I am guessing that some packages just added a few extra transitive dependencies and it managed to tip the balance.

I guess the best possible approach here is to find the reason behind the x3 flag amplification.

UPDATE: It seems this is not even a x3 amplification, it depends in the chain of dependencies I managed to even get up to x5 amplification of flags.

@liarokapisv
Copy link
Contributor Author

It appears that the amplification happens due to cc-wrapper setup hooks being injected at different levels. Specifically the problem seems to appear due to clang being used for the various ament tools.

See relevant issue at NixOS/nixpkgs#364984.

@hacker1024
Copy link
Contributor

To my understanding, we end up with many propagatedBuildInputs because it's often impossible to know if a dependency actually needs to be propagated or not. REP 149 actually defines a wide range of dependency types that can map pretty nicely to Nixpkgs's, but a significant amount of packages use the basic depend tag, which is not helpful.

(See here for the current Nix generation logic.)

I wonder how necessary this actually is. What would happen if we tried to use buildInputs instead? If there are certain common dependencies that do need to be propagated, could we get away with maintaining a list of them and only propagating packages from that whitelist by default? At the very least, we can use a blacklist - I doubt rclcpp ever needs to be propagated, for example, but it often is.

I also wonder if we can handle the exec_depend group a little better:

The <exec_depend> is needed for packages providing shared libraries, executable commands, Python modules, launch scripts or any other files required for running your package. It is also used by metapackages for grouping packages.

At the moment, we add exec_depend to nativeBuildInputs. If you think about it, though:

  • Shared libraries will be linked to directly in the far majority of cases, so they're fine in buildInputs alone. We can default to this.
  • Executable commands can be provided through wrappers adding to PATH instead of generic propagation. These packages can be identified by scanning their bin directories for executables.
  • Python modules are a legitimate usecase, but we can identify them pretty easily and add them to propagatedBuildInputs (or the new dependencies attribute`) as normal.
  • Launch scripts are harder, but they aren't used during building. We can add ROS packages to a different list in passthru instead that is then used by buildEnv directly.

This exec_depend thing is really the main point of friction between REP 149 and Nixpkgs. On a regular Linux system, shared libraries are runtime dependencies that must be installed alongside the intended package. With Nix, that happens automatically, so they're a buildInput and not a propagatedBuildInput. Executable dependencies are similar (included in wrappers, so a buildInput and not a propagatedBuildInput).

I wonder if we can convince the ROS package manifest format team to make some changes that would help us and improve the ecosystem.

  • Split exec_depend into its four categories: exec_lib_depend, exec_bin_depend, exec_python_depend, exec_ros_depend
  • Deprecate the generic depend so that package maintainers become more granular in their package.xmls

@liarokapisv
Copy link
Contributor Author

liarokapisv commented Dec 18, 2024

I have to revisit this but I think that the main issue with buildInputs is config & header files.

In general buildInputs corresponds to PRIVATE dependencies. This means that neither headers or cmake / other build-system specific config files are exposed to subsequent dependent packages.

When a library B has publicly used headers of library A, and a library C needs B, it needs to not only be linked to B but also have access to A's headers /for the build/, hence it needs a nativeBuildInputs dependency to A.

When using buildInputs for a dependency, headers are not propagated.
In the same vein, if a library uses find_package on it's cmake config file, this depends on the dependency's config file being present in NIXPKGS_CMAKE_PREFIX_PATH. Again, with buildInputs these are not exposed to subsequent dependent packages so this will fail.

I think a workable alternative is to use buildInputs for dependencies and then also expose these to subsequent dependent packages with propagatedNativeBuildInputs. This will properly expose the build-tool specific config files and the headers but will not propagate the libs, which will be directly embedded in the elf files. This /will/ fail for module-like libraries which need to be dlopen-ed.

EDIT: This won't work because propagatedNativeBuildInputs will only work for the direct consumer packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants