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

reword documentation on nix-store --import #10712

Merged

Conversation

fricklerhandwerk
Copy link
Contributor

  • add links to definitions of terms
  • one sentence per line
  • be more specific about which store is used for the import
  • clearly distinguish store paths and store objects
  • make a recommendation to use nix-copy-closure for efficient SSH transfers

Motivation

reworking documentation around use cases related to nix-copy-closure, as discussed with @eflanagan0

Context

related: #10708

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

- add links to definitions of terms
- one sentence per line
- be more specific about which store is used for the import
- clearly distinguish store paths and store objects
- make a recommendation to use `nix-copy-closure` for efficient SSH transfers
@edolstra
Copy link
Member

It's probably a good idea to add a warning that nix-store --import / --export is deprecated since it doesn't copy some necessary metadata (like the CA field).

@fricklerhandwerk
Copy link
Contributor Author

Is the CA field necessary outside of the experimental feature? Because if not, that would be a bug in CA xor one could say that support for nix-store --export / --import is not in scope for CA. Deprecating would be odd given there is no stable replacement with feature parity.

Are there any other metadata from stable functionality that are not preserved?

@edolstra
Copy link
Member

edolstra commented May 15, 2024

Yes, the CA field is stable and not directly related to ca-derivations.

See also #9474 (comment).

@fricklerhandwerk
Copy link
Contributor Author

TBH I'm don't know in what way the command is broken, what the CA fields are for, and what happens if they are missing. I'd be glad to document that if you can point me to the relevant test cases or code. But the straightforward example in this PR works - is there any reason not to merge that?

@Ericson2314 Ericson2314 merged commit bbe780b into NixOS:master May 15, 2024
9 checks passed
@Ericson2314
Copy link
Member

@fricklerhandwerk the details of the CA field I don't think matter for this. What is going on is:

  1. The import/export format lacks versioning
  2. The (stable features only even) metadata has changed since import/export was created
  3. import/export is thus lacking functionality in a number of hard-to-describe ways.

Exhaustively figuring out all the missing functionality would be hard, but we can say something like

These commands are deprecated because they use an protocol that cannot be versioned/modified. As such, they transmit/receive via an obsolete encoding the store object metadata, causing store object written on the destination side to potentially differ from their counterparts on the source side in subtle ways. Please instead use... which does not have this deficiency.

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

Successfully merging this pull request may close these issues.

None yet

3 participants