Skip to content

Conversation

thufschmitt
Copy link
Member

@thufschmitt thufschmitt commented Nov 27, 2023

Motivation

The only thing that the issue didn't precise is how we should call the
current (nix-store --export) format1. I've settled to binary by lack
of something better.

Context

Fix #9038

Priorities

Add 👍 to pull requests you find important.

Footnotes

  1. The issue mentions --format tar, but this is whishful thinking
    for the future, the current format isn't tar-based at all.

@github-actions github-actions bot added documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Nov 27, 2023
Fix NixOS#9038

The only thing that the issue didn't precise is how we should call the
current (`nix-store --export`) format[^1]. I've settled to `binary` by lack
of something better.

[^1]: The issue mentions `--format tar`, but this is whishful thinking
for the future, the current format isn't `tar`-based at all.
@roberth roberth changed the title Add a nix store import and nix store export commands Add nix store import and nix store export commands Nov 27, 2023
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

Did a bit of digging to help with naming and documentation.

This should have a page in the protocols section of the manual. Basic info:

Framing protocol

A binary protocol for a stream of store objects. It has framing which does not allow trivial concatenation of streams - the end of stream marker would have to be stripped from the input streams.
Contents:

  • no magic bytes in
  • unit64 00 00 00 00 00 00 00 00 indicates end of stream
  • uint64 01 00 00 00 00 00 00 00 indicates that a store object follows

Store object

Can not be differentiated from a NAR, except by checking whether the stream ends after the NAR has been parsed.

  • no magic bytes
  • a NAR
  • other fields (details tbd)

NAR

NAR has magic bytes. Currently only the nix-archive-1 format exists.


StorePathSet pathsAsSet;
pathsAsSet.insert(storePaths.begin(), storePaths.end());
FdSink sink(STDOUT_FILENO);
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that in that case we don't really care about the progress bar as it's not expected that the thing will be printed on the terminal (because it's a binary blob).
However, we should probably not print anything if stdout is a tty. I'll see to that

Copy link
Member Author

Choose a reason for hiding this comment

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

I've made it barf when trying to write to a tty, unless explicitly requested

)",
.labels = {"format"},
.handler = {[&](std::string_view arg) {
if (arg == "binary")
Copy link
Member

Choose a reason for hiding this comment

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

Ideally the format identifier is usable out of context, because it's more recognizable and searchable that way.
E.g.

Suggested change
if (arg == "binary")
if (arg == "nix-store-object-stream-0")

thought: For version 1 it would be wise to add magic bytes. That would bring it up to NAR level, which has nix-archive-1 for magic.

Copy link
Member Author

Choose a reason for hiding this comment

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

Erk, I really wouldn't want to have to write nix store export --format nix-store-object-stream-0 😬 (at least not if there's no alternative)

Copy link
Member

Choose a reason for hiding this comment

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

Maybe binary should be an alias for this format name, at which point we're almost full circle. I think that's good?

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk Nov 27, 2023

Choose a reason for hiding this comment

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

We could lay groundwork to be able to say in the future something like --format binary --format-version 1 (default to latest). But I agree with @roberth that there should be a "fancy" or fully-qualified, distinctive name that is easy to find when one needs to figure things out.

Copy link
Member Author

@thufschmitt thufschmitt Nov 29, 2023

Choose a reason for hiding this comment

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

Well, we can add that, but isn't that vastly overkill and confusing for something that we most likely won't want to evolve? We might add new formats, which is why we settled on having this --format parameter, but if we ever add another one, that'll most likely be something totally different. And I really don't see anyone willing to pass --format nix-store-object-stream-0

Copy link
Contributor

Choose a reason for hiding this comment

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

I can imagine this, but actually we can add the behavior I proposed exactly when needed. Therefore nevermind.

StorePathSet pathsAsSet;
pathsAsSet.insert(storePaths.begin(), storePaths.end());
FdSink sink(STDOUT_FILENO);
store->exportPaths(pathsAsSet, sink);
Copy link
Member

Choose a reason for hiding this comment

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

Now that we've named the protocol, we can mention it in the exportPaths method's header, and link to the manual.

Allow specifying a file in `nix store export` and `nix store import`.

Also refuse to write binary things to the terminal unless explicitly
asked to.
@edolstra
Copy link
Member

edolstra commented Nov 29, 2023

IMHO we should not have the import/export format in the new CLI at all. It's poorly designed (see #3183 (comment)), not extensible, currently broken (in that it doesn't forward CA fields and signatures), and AFAIK nobody uses it anymore. This to me is a great example of historical cruft in the old CLI that we should not carry forward into the new CLI.

* Export the closure of a given installable and re-import it in another machine

```console
$ nix store export --recursive --format binary nixpkgs#hello > hello-closure.tar
Copy link
Member

Choose a reason for hiding this comment

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

This suggests that the output is a tarball, which is not the case.

@thufschmitt
Copy link
Member Author

IMHO we should not have the import/export format in the new CLI at all

I would be totally fine with it.

This leaves us with two options:

  1. Don't add the command at all. This would remove some functionality, although it's not clear whether that's useful functionality – essentially the only thing that we would loose would be the ability to export something that's not a closure since exporting a closure can be done with nix copy
  2. Add it, but with another, nicer, format. A tar or zip (better because of random-access?) archive containing something in the format of a binary cache store would probably tick all the boxes.

I'd be in favour of 1., and only do 2. lazily if people ask for it (or even just let people implement it if they feel like it)

@7c6f434c
Copy link
Member

Is it possible (from CLI) to export/import a single pair of .nar+.narinfo? Incrementality has been useful in some cases, the format of stuff on top of NAR seems to be a minor thing.

@roberth
Copy link
Member

roberth commented Nov 29, 2023

  1. Don't add the command at all. This would remove some functionality, although it's not clear whether that's useful functionality – essentially the only thing that we would loose would be the ability to export something that's not a closure since exporting a closure can be done with nix copy

I suppose the functionality is the format being a simple byte stream, which can be handled more freely than nix copy which has to be aware of the protocol for data transfer, for instance.

2. zip (better because of random-access?)

The zip header is a blessing and a curse. It's only at the end, so you have to first store the whole thing in a non-final way before you have the necessary info to put it into a store properly. Not great for importing. A tar of individually compressed files would be pretty good. Actually a tar of a binary cache directory would give you that.
This is also more extensible than the current binary format, although the protocol has room for extension thanks to how it encodes the sequence of store objects (ie "Framing" earlier). Not the best forward compatibility, but actually not bad.

$ printf '\x02\x00\x00\x00\x00\x00\x00\x00' | nix-store --import
error: input doesn't look like something created by 'nix-store --export'

There's your version 2 binary format ;)

[assorted complaints about the export format]

If we're going to do new format, we'd better start from the bottom.

  • NAR: definitely need to keep, because it is used in hashing.
  • narinfo: name is not appropriate for what it is. It describes a store object (not just a file system object, ie NAR) but also cache specific info such as .nar location. Something in JSON format would be nice.

@thufschmitt
Copy link
Member Author

Is it possible (from CLI) to export/import a single pair of .nar+.narinfo? Incrementality has been useful in some cases, the format of stuff on top of NAR seems to be a minor thing.

Not in one go, but nix nar pack + nix path-info --json essentially gives you that, yes. Not in a way that can easily be reimported though afaik.

@7c6f434c
Copy link
Member

If it cannot be reimported, it doesn't provide the valuable part of nix-store --export/--import, though…

@edolstra
Copy link
Member

edolstra commented Dec 1, 2023

@thufschmitt I agree with lazily doing option 2. We can (eventually) add these commands when somebody comes up with a better closure serialization format.

@7c6f434c
Copy link
Member

7c6f434c commented Dec 1, 2023

closure serialization

This is a loss of functionality; for a closure one can tar/dat/whatever a nix copy-made cache; import/export are useful for incremental import/export (when direct network connection is inconvenient, or to wipe-and-recover with no need to backup stuff already available elsewhere)

@roberth
Copy link
Member

roberth commented Dec 4, 2023

Another protocol that solves the extensibility problem is half a version handshake and then WorkerProto::Op::AddMultipleToStore.

The quality of this solution exist in a superposition of pretty ok and absolutely terrible. Not sure.

@x10an14
Copy link

x10an14 commented Feb 26, 2024

Hey! Just spoke w/ @tomberek about the lack of nix-store --import's equivalent in the new experimental nix commands set.

Our use-case is leveraging a OCI (read: OCI container image registry) to upload NAR files (ref: oras.land), and thus sharing/distributing/caching builds.

@Reasonable-Solutions can perhaps expand a bit more on this/explain why this is a route that we're following (I cannot) =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add nix store import and nix store export commands to work with nar/narinfo

6 participants