Set strictDeps=true on our top level packages #192

TL;DR: I think it's preferable to enforce strictDeps so our derivations are correct before wasting time and resources cross-compiling something that we could already have known would fail beforehand.

> I was planning to add another job to the CI to run the cross compilation target. Wouldn't that cover what this patch is trying to solve? We already discussed it bscpkgs: https://jungle.bsc.es/git/rarias/bscpkgs/pulls/10#issuecomment-4209 TL;DR: I think it's preferable to enforce `strictDeps` so our derivations are correct before wasting time and resources cross-compiling something that we could already have known would fail beforehand.

rarias commented

2025-10-09 13:38:54 +02:00

I was planning to add another job to the CI to run the cross compilation target. Wouldn't that cover what this patch is trying to solve?

We already discussed it bscpkgs: rarias/bscpkgs#10 (comment)

TL;DR: I think it's preferable to enforce strictDeps so our derivations are correct before wasting time and resources cross-compiling something that we could already have known would fail beforehand.

I have two issues with strictDeps:

Is not well documented so I don't know if it does only what you think it does. This can be resolved by reviewing the relevant parts of the stdenv machinery (and ideally having some notes for the future).
I don't like auto-enabling this when is not done in upstream yet (this is partially caused by the point above). Maybe a less risky option is to fail to evaluate if you forgot to set it either to true of false, so you have to explictly set it in all derivations.

If you think it is still useful to have it, I would need to review how it works internally. If that point is clear, I won't oppose enabling it for those derivations that you have worked on. Not sure if we want to enable it for all where is not set.

> > I was planning to add another job to the CI to run the cross compilation target. Wouldn't that cover what this patch is trying to solve? > > We already discussed it bscpkgs: https://jungle.bsc.es/git/rarias/bscpkgs/pulls/10#issuecomment-4209 > > > TL;DR: I think it's preferable to enforce `strictDeps` so our derivations are correct before wasting time and resources cross-compiling something that we could already have known would fail beforehand. I have two issues with strictDeps: 1. Is not well documented so I don't know if it does only what you think it does. This can be resolved by reviewing the relevant parts of the stdenv machinery (and ideally having some notes for the future). 2. I don't like auto-enabling this when is not done in upstream yet (this is partially caused by the point above). Maybe a less risky option is to fail to evaluate if you forgot to set it either to true of false, so you have to explictly set it in all derivations. If you think it is still useful to have it, I would need to review how it works internally. If that point is clear, I won't oppose enabling it for those derivations that you have worked on. Not sure if we want to enable it for all where is not set.

abonerib commented

2025-10-09 14:54:55 +02:00

Is not well documented so I don't know if it does only what you think it does. This can be resolved by reviewing the relevant parts of the stdenv machinery (and ideally having some notes for the future).

I have taken a look in stdenv and there the two takeaways are:

When cross-compiling, by default it is enabled: f00b180ed4/pkgs/stdenv/generic/make-derivation.nix (L225)
- This has the unfortunate consequence that setting strictDeps = false will affect cross compilation (I may be wrong)
The actual logic is in the stdenv setup.sh: https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/setup.sh#L850
- There are only 3 uses of strictDeps. From what I could understand:
  - 2 of them related to skip adding things to the HOST_PATH and
  - the other skips running setup hooks from activated packages. The setup hooks could do anything, so who knows what the consequences are.

I don't like auto-enabling this when is not done in upstream yet (this is partially caused by the point above). Maybe a less risky option is to fail to evaluate if you forgot to set it either to true of false, so you have to explictly set it in all derivations.

I think that explicitly setting it to false could also have unintended consequences (see above).

If you think it is still useful to have it, I would need to review how it works internally. If that point is clear, I won't oppose enabling it for those derivations that you have worked on. Not sure if we want to enable it for all where is not set.

I agree that adding it by default magically to all our packages is a bit of a hack. Maybe we can add it as a ci target, but that would mean a lot of rebuilds.

> 1. Is not well documented so I don't know if it does only what you think it does. This can be resolved by reviewing the relevant parts of the stdenv machinery (and ideally having some notes for the future). I have taken a look in stdenv and there the two takeaways are: - When cross-compiling, by default it is enabled: https://github.com/NixOS/nixpkgs/blob/f00b180ed496b11337983fe4b0b979419cb3e472/pkgs/stdenv/generic/make-derivation.nix#L225 - This has the unfortunate consequence that setting `strictDeps = false` will affect cross compilation (I may be wrong) - The actual logic is in the stdenv setup.sh: https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/setup.sh#L850 - There are only 3 uses of strictDeps. From what I could understand: - 2 of them related to skip adding things to the HOST_PATH and - the other skips running setup hooks from activated packages. The setup hooks could do anything, so who knows what the consequences are. > 2. I don't like auto-enabling this when is not done in upstream yet (this is partially caused by the point above). Maybe a less risky option is to fail to evaluate if you forgot to set it either to true of false, so you have to explictly set it in all derivations. I think that explicitly setting it to false could also have unintended consequences (see above). > If you think it is still useful to have it, I would need to review how it works internally. If that point is clear, I won't oppose enabling it for those derivations that you have worked on. Not sure if we want to enable it for all where is not set. I agree that adding it by default magically to all our packages is a bit of a hack. Maybe we can add it as a ci target, but that would mean a lot of rebuilds.

CI / build:all (pull_request) Successful in 15s

Details

This pull request has changes conflicting with the target branch.

overlay.nix

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin enableStrictDeps:enableStrictDeps

git checkout enableStrictDeps

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: rarias/jungle#192