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

Add crates overrides #298

Open
wants to merge 5 commits into
base: unstable
Choose a base branch
from

Conversation

litchipi
Copy link

@litchipi litchipi commented Oct 3, 2022

Context: I'm trying to build a Tauri application using Nix.
All these crates requires an overlay, and I thought it would be preferable to add them directly there.
I copied the format used in the file, and it works well like this, feel free to edit it as you want.

Does this file require to be sorted alphabetically ?

psionic-k and others added 4 commits October 1, 2022 23:29
A bit of history.  The workspace shell started off as a quick solution to get
overrides to express some of their side effects within the user's development
shell.  Because it didn't work closely with the actual mechanisms of mkShell,
the chosen mechanism, from overrides to dev shell, was use of propagated
inputs.  This and some cargo culting had resulted in overrides mimicking other
overrides, lots of use of propagatedNativeBuildInputs.

First, workspace shell's overhaul is given the actual package set instead of the
noBuild set that is used when running tests.  (the tests themselves may need
similar overhaul).  The new shell has one simple job:  Make everything required
to build every crate, except rust crates themselves, expressed in environment
and available from the store.  This has to be done for the entire rust crate
DAG.

Second, once the workspace shell could correctly express dependencies and shell
hooks etc without the abuse of propagated inputs, the overrides themselves could
be made much more sensible, much more consistent with the rest of Nix.

substractLists works fine for derivation filtering

The issue that lead to using attrSets was using the wrong crate outputs to
filter on.  After that was fixed, it was no longer necessary to construct
attrSets by keys.  If complexity of a dependency set (n^2 because of lib.unique)
becomes too high, the attrSet based implementation should become favored again.
This won't really show up to users for n < 1000 where n is the immediate
dependencies of crates that are not crates themselves.

Apple frameworks must be propagated?  Missing a setupHook?
If an explicit workspace is configured in a crate's manifest, it will brake when
we move that manifest out of the way for indpendent crate building.
This only works with nightly toolchains.  Likely doesn't work at all whenever
the flag should affect dependencies, which cargo cannot access / decide within
the sandbox
@litchipi litchipi changed the base branch from release-0.11.0 to unstable October 3, 2022 15:14
@psionic-k
Copy link
Member

Sure, let's sort alphabetically. I would take a look at if your dependencies will work as just build inputs. It's not usually the case that they need to propagate.

pkg-config from nixpkgs will usually be transitively included because of a crate depending on the pkg-config crate. This is an example where a dependency must be propagated. For normal libraries, it's usually fine to leave them in buildInputs.

Apple frameworks for some reason still require propagation, perhaps a linking style issue.

You can continue committing on this branch and I will cherry pick over your commits and handle manual conflict. Attribution is retained.

@psionic-k
Copy link
Member

I force pushed the file you are working on in #292

These crates are necessary for building a Tauri application

Signed-off-by: Litchi Pi <[email protected]>
@litchipi
Copy link
Author

litchipi commented Oct 5, 2022

I tried to have the minimal amount of propagated inputs, and this setup should now work !
Also I squashed and rebased over release-branch-maintenance so it should be cherry-pickable if you want

@psionic-k
Copy link
Member

LGTM. I'll handle any nits if any come up. Will be in unstable and 0.12 (doesn't exist yet) after close

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

Successfully merging this pull request may close these issues.

None yet

3 participants