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

log all ifd #3494

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

log all ifd #3494

wants to merge 2 commits into from

Conversation

cleverca22
Copy link
Contributor

No description provided.

@@ -56,6 +56,9 @@ void EvalState::realiseContext(const PathSet & context)
throw InvalidPathError(store->printStorePath(ctx));
if (!decoded.second.empty() && ctx.isDerivation()) {
drvs.push_back(StorePathWithOutputs{ctx.clone(), {decoded.second}});
if (evalSettings.logAllIFD) {
printInfo(format("%1% depends on %2% via %3%") % pos % decoded.first % reason);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe something a bit more explicit, mentioning "importing from derivation"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also in the help text please.

src/libexpr/eval.hh Outdated Show resolved Hide resolved
@grahamc
Copy link
Member

grahamc commented Apr 14, 2020

One last nit, and looks great to me

@johnalotoski johnalotoski force-pushed the log-all-ifd branch 2 times, most recently from d43b06d to 2c36d22 Compare April 14, 2020 19:39
@edolstra
Copy link
Member

What's the use case for this? Why does it need a separate option rather than a debug(...) message?

@grahamc
Copy link
Member

grahamc commented Apr 14, 2020

debug sounds like a fine option, the main thing is being able to easily see the origin of the IFD

@cleverca22
Copy link
Contributor Author

cleverca22 commented Apr 15, 2020

if it was done via debug(...) then you would need enough -vvvv (which can harm performance?) and know the magic string to grep for, and grep on stderr is more complex

having a dedicated option eliminates the need to know the magic string to search for

i can also see value in just turning this option on globally, so you have more feedback about why something just started a build when it was doing an eval

src/libexpr/eval.hh Outdated Show resolved Hide resolved
src/libexpr/primops.cc Outdated Show resolved Hide resolved
@Ericson2314
Copy link
Member

Ericson2314 commented Apr 15, 2020

What's the use case for this?

I just discovered this. this would be very useful to us making sure we have all build-time and eval-time deps, when trying to export caches for clients. Definitely do want!

@matthewbauer
Copy link
Member

matthewbauer commented Apr 16, 2020

Debugging this stuff seems useful, but I wonder if we could have a flag that gives a machine readable output? A common reason to want to know about all of these .drv files is so that you can keep a cache, not just of the dependencies of your final drv, but also of the dependencies of the imported drv.

For this, I would propose having an option to nix-instantiate like this:

$ nix-instantiate --include-ifd my-imported-drv.nix
/nix/store/00000000000000000000000000000000-my-drv.drv
/nix/store/00000000000000000000000000000000-my-imported-drv.drv

This way you could pipe it to something like cachix with:

$ nix-store -qR --include-outputs $(nix-instantiate --include-ifd my-imported-drv.nix) | cachix push my-cache

and have all dependencies cached. If this sounds like a good idea, I'd be happy to take a stab at implementing it.

/cc @domenkozar

@domenkozar
Copy link
Member

Hell yes, that would really help with syncing to binary caches!

@disassembler
Copy link
Member

@edolstra what's needed to get this done? It seems like a machine readable option is a bonus unrelated to just getting the logging. Do you think this could be merged as-is and @matthewbauer could put his new functionality in a follow-up?

@deepfire
Copy link

@grahamc, what do you think needs to be done (besides a rebase)?

@bqv
Copy link
Contributor

bqv commented Feb 4, 2021

Tumbleweed?

src/libexpr/primops.cc Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Oct 30, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Oct 30, 2021
@cleverca22
Copy link
Contributor Author

not stale, i do intend to finish this at some point

@deepfire
Copy link

deepfire commented Feb 3, 2022

This works -- for example, in recent cardano-node:

[user@machine:~/cardano-node]$ nix flake show --option log-all-ifd true --option allow-import-from-derivation true -v
warning: unknown experimental feature 'ca-references'
warning: Git tree '/home/user/cardano-node' is dirty
evaluating ''...
git+file:///home/user/cardano-node
evaluating 'apps'...
├───apps
evaluating 'apps.x86_64-darwin'...
│   ├───x86_64-darwin
trace: WARNING: 8.10.5 is out of date, consider using 8.10.7.
trace: haskell-nix.haskellLib.cleanGit: /nix/store/q9lj3lzf4khxhl2w0mzhni2w190gv47w-source does not seem to be a git repository,
assuming it is a clean checkout.
trace: To make project.plan-nix for cardano-node a fixed-output derivation but not materialized, set `plan-sha256` to the output of the 'calculateMaterializedSha' script in 'passthru'.
trace: To materialize project.plan-nix for cardano-node entirely, pass a writable path as the `materialized` argument and run the 'updateMaterialized' script in 'passthru'.
trace: WARNING: 8.10.5 is out of date, consider using 8.10.7.
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/import-and-filter-project.nix:21:13 importing from derivation /nix/store/vnkwjg5aq1jkqy3cfjf6xlmz1ba8fp32-nix-tools-plan-to-nix-pkgs.drv via scopedImport
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/spdx/licenses.nix:6:8 importing from derivation /nix/store/dscrvrxsk3sc6q86vix26sdd4k7w3l09-spdx-license-list-data-3.12.drv via readFile
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/import-and-filter-project.nix:21:13 importing from derivation /nix/store/l6zrcg2d26n7gjyzsz5dqlp6xhdrl0ji-alex-plan-to-nix-pkgs.drv via scopedImport
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/spdx/licenses.nix:6:8 importing from derivation /nix/store/dscrvrxsk3sc6q86vix26sdd4k7w3l09-spdx-license-list-data-3.12.drv via readFile
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/import-and-filter-project.nix:21:13 importing from derivation /nix/store/2k63z16v1kphs60w763i9n4326sl34n9-happy-plan-to-nix-pkgs.drv via scopedImport
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/spdx/licenses.nix:6:8 importing from derivation /nix/store/dscrvrxsk3sc6q86vix26sdd4k7w3l09-spdx-license-list-data-3.12.drv via readFile
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/import-and-filter-project.nix:21:13 importing from derivation /nix/store/5dm6ijxsv3b5mga4bjqx2vzgdv4k2sfa-hscolour-plan-to-nix-pkgs.drv via scopedImport
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/spdx/licenses.nix:6:8 importing from derivation /nix/store/dscrvrxsk3sc6q86vix26sdd4k7w3l09-spdx-license-list-data-3.12.drv via readFile
/nix/store/jr8mn1grmhlp707j02hnm8pllr89pbsi-source/lib/import-and-filter-project.nix:21:13 importing from derivation /nix/store/hkvscbssj0cmkl5gl15xn84zrr5l3xmm-happy-plan-to-nix-pkgs.drv via scopedImport

src/libexpr/eval.hh Outdated Show resolved Hide resolved
src/libexpr/eval.hh Outdated Show resolved Hide resolved
src/libexpr/primops.cc Outdated Show resolved Hide resolved
src/libexpr/primops.cc Outdated Show resolved Hide resolved
src/libexpr/primops.cc Outdated Show resolved Hide resolved
@cidkidnix
Copy link
Contributor

I've fixed/updated this PR/patch to work on Nix 2.9+ if anyone is interested
https://github.com/cidkidnix/nix/tree/log-ifd

@stale stale bot removed the stale label Oct 12, 2022
@fricklerhandwerk fricklerhandwerk added error-messages Confusing messages and better diagnostics UX The way in which users interact with Nix. Higher level than UI. labels Mar 3, 2023
@thufschmitt thufschmitt added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Mar 17, 2023
@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Mar 22, 2023

Triaged in the Nix team meeting 2023-03-17:

The team was concerned it be too verbose, but since it's hidden behind a flag, should be OK. The idea is approved and we will shepherd the implementation to completion.

@cleverca22 are you still interested in finishing the PR?
In case not, @cidkidnix since you've rebased this somewhat recently (on Nix development time scales), would you be able to take it over?

@cidkidnix cidkidnix mentioned this pull request Mar 22, 2023
7 tasks
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-03-17-nix-team-meeting-minutes-41/26614/1

@Ericson2314
Copy link
Member

I am going to mark this draft because it is old with conflicts, and also because I suspect the rebase in #8094 will get there first.

@Ericson2314 Ericson2314 marked this pull request as draft May 13, 2023 13:17
This will probably break it, but we can cherry-pick to rebase.

Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-messages Confusing messages and better diagnostics idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. UX The way in which users interact with Nix. Higher level than UI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.