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.

Sign up for GitHub

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

Jump to bottom

Fixes #94: do not include config file in nix database. #96

Merged
merged 3 commits into from
Nov 19, 2023
Merged

Fixes #94: do not include config file in nix database. #96

merged 3 commits into from
Nov 19, 2023

Conversation

kolloch
Copy link
Contributor

@kolloch kolloch commented Nov 6, 2023

closureGraph is extended with an ignore argument which will filter out the config file when needed.

The resulting graph is both passed to nix2container-bin and makeNixDatabase -- ensuring that both contain the same paths.

The --ignore flag of the nix2container-bin is not used anymore.

#94

@kolloch kolloch changed the title Fix: do not include config file in nix database. Fixes #94: do not include config file in nix database. Nov 6, 2023
@kolloch
Copy link
Contributor Author

kolloch commented Nov 6, 2023

I noticed the the closure-graphs end up in the image in addition to what also was added before:

  • layers.json (only needed for build, right?)
  • perms derivations (even though also copied to root)
  • root-links (even though also copied to root)

@kolloch kolloch mentioned this pull request Nov 7, 2023
Closed
@kolloch kolloch force-pushed the fix/94-closure branch 2 times, most recently from 5fa286e to d554a88 Compare November 10, 2023 17:55
Fixes nlewo#94: exclude config file and json files from nix database.
6b39518 
`closureGraph` is extended to ignore all paths passed in the new
ignore arguement.

The resulting graph is both passed to nix2container-bin and
makeNixDatabase -- ensuring that both contain the same paths.

makeNixDatabase is rewritten, such that it always expects a
closureGraphJson as arguement but does not include it in the database.

The `--ignore` flag of the nix2container-bin is not used anymore.

nlewo#94
@kolloch kolloch force-pushed the fix/94-closure branch 4 times, most recently from 6ecbcc7 to 3e3bb62 Compare November 10, 2023 19:46
[nix-database] make deterministic
a69c994 
Sorting inputs, redumping DB
@kolloch
Copy link
Contributor Author

kolloch commented Nov 10, 2023

Hi @nlewo,

I extended this quite a lot:

  • closure-graph.json and layer.json files are now excluded from the resulting container.
  • closureGraph is now deterministic in my limited test cases.
  • makeNixDatabase is now deterministic in my limited test cases.

Question: Not sure if I need to handle /nix/store/.links in a special fashion.

Now my test case works reliably with reproducible=false but not if I have reproducible=true.

If that isn't ironic!

@nlewo
Copy link
Owner

nlewo commented Nov 19, 2023

Thanks @kolloch

This LGTM (but i'm not 100% confident since these parts are not really we tested...).

Now my test case works reliably with reproducible=false but not if I have reproducible=true.

It means there are still some data that are not reproducible.
Could you share a command to reproduce this? I could then take a look at the built image. (I know you set up a repository to reproduce your issue, but after all your changes and fixes, i'm a bit confused.)

BTW, these changes improve the current state. So, even if they are not perfect yet, we can merge them.

@nlewo nlewo merged commit 4400b77 into nlewo:master Nov 19, 2023
3 checks passed
@kolloch
Copy link
Contributor Author

kolloch commented Nov 19, 2023

Hi @nlewo, thanks for the merge!

I tested so many things, I am also confused ;)

I am not at my work machine right now where I reproduced the error last. I tried to reproduce it on an Ubuntu WM and couldn't anymore. Maybe it is good now?

According to my notes, this should have reproduced the error:

nix run .\#x86_64-linux.gitlab.runnables.debug-building-stdenv-only-in-nix-ci

in https://gitlab.com/nexxiot-labs/nix2container-checksum

But for me it just works now. Did we fix it completely by accident? Or was there some weird store path thing on my work machine? I don't know...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kolloch @nlewo