Skip to content

Add documentation for NAR spec in kaitai #14345

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
Merged
merged 1 commit into from
Nov 3, 2025
Merged

Add documentation for NAR spec in kaitai #14345

merged 1 commit into from
Nov 3, 2025

Conversation

@fzakaria
Copy link
Contributor

@fzakaria fzakaria commented Oct 23, 2025 •
edited
Loading

Motivation

Feedback from @roberth was to contribute the spec to the documentation.

The spec originates from https://github.com/fzakaria/nix-nar-kaitai-spec which includes some tests to validate that it's correct. It's not exhaustive but the tests consider enough cases to believe the spec is robust.

Add 👍 to pull requests you find important.

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

@fzakaria fzakaria requested a review from edolstra as a code owner October 23, 2025 21:19
cole-h
cole-h reviewed Oct 23, 2025
View reviewed changes
Ericson2314
Ericson2314 reviewed Oct 23, 2025
View reviewed changes
Ericson2314
Ericson2314 reviewed Oct 23, 2025
View reviewed changes
xokdvium
xokdvium reviewed Oct 24, 2025
View reviewed changes
@github-actions github-actions bot added the new-cli Relating to the "nix" command label Oct 24, 2025
@fzakaria fzakaria force-pushed the fzakaria/nar-kaitai-spec branch from 8a1ad7d to 5ae653a Compare October 24, 2025 04:09
@Ericson2314
Copy link
Member

Ericson2314 commented Oct 24, 2025

The last thing: can you mimick https://github.com/NixOS/nix/tree/master/src/json-schema-checks (grep for json-schema-checks) and make a kataistruct-checks which will verify that our example NARs in the test suite (I think src/libutil-test/data) are valid according to the spec? (and if we have some invalid NARs, likewise test that are not valid according to the spec.)

@fzakaria
Copy link
Contributor Author

fzakaria commented Oct 24, 2025

@Ericson2314 ... that took a little bit but i added the test suite as requested. :)

cole-h
cole-h reviewed Oct 24, 2025
View reviewed changes
cole-h
cole-h reviewed Oct 24, 2025
View reviewed changes
@nixos-discourse
Copy link

nixos-discourse commented Oct 24, 2025

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

https://discourse.nixos.org/t/path-hasher-a-php-implementation-of-nix-s-nar-format/71222/2

drupol
drupol reviewed Oct 24, 2025
View reviewed changes
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

Minor nits

@fzakaria fzakaria requested a review from cole-h October 25, 2025 00:03
@Ericson2314
Copy link
Member

Ericson2314 commented Oct 25, 2025

Sorry for the yak shaving requests but I would like for:

  1. Wait for Nixpkgs PR to land upstream (seems very close, I just left one more thing)
  2. Backport that PR as needed (I can do that if it's a matter of perms)
  3. Bump Nixpkgs in this flake, don't need to wait for hydra, can use release-* for example for now
  4. This PR doesn't need to vendor package

It's a lot of manual steps, but hopefully not too much waiting since don't need to wait for Hydra.

@fzakaria
Copy link
Contributor Author

fzakaria commented Oct 25, 2025

Sorry for the yak shaving requests but I would like for:

  1. Wait for Nixpkgs PR to land upstream (seems very close, I just left one more thing)
  2. Backport that PR as needed (I can do that if it's a matter of perms)
  3. Bump Nixpkgs in this flake, don't need to wait for hydra, can use release-* for example for now
  4. This PR doesn't need to vendor package

It's a lot of manual steps, but hopefully not too much waiting since don't need to wait for Hydra.

Spoke over Matrix.

I will do 1., 3. and 4. -- @Ericson2314 will help me with 2. as I'm inexperienced with backporting and don't have the permissions.

cole-h
cole-h approved these changes Oct 25, 2025
View reviewed changes
Copy link
Member

@cole-h cole-h left a comment

Choose a reason for hiding this comment

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

Thanks for dealing with my nits!

Ericson2314
Ericson2314 reviewed Oct 25, 2025
View reviewed changes
Ericson2314
Ericson2314 reviewed Oct 25, 2025
View reviewed changes
@fzakaria fzakaria requested a review from Ericson2314 October 26, 2025 17:29
@fzakaria fzakaria force-pushed the fzakaria/nar-kaitai-spec branch 2 times, most recently from 3ea9b34 to 17aef29 Compare October 27, 2025 03:44
Ericson2314
Ericson2314 reviewed Oct 27, 2025
View reviewed changes
Ericson2314
Ericson2314 reviewed Oct 27, 2025
View reviewed changes
@fzakaria fzakaria force-pushed the fzakaria/nar-kaitai-spec branch 2 times, most recently from c4c11c4 to 6a1b5c5 Compare October 27, 2025 18:32
@fzakaria fzakaria requested a review from Ericson2314 October 27, 2025 18:32
@fzakaria
Copy link
Contributor Author

fzakaria commented Oct 27, 2025

Cleaned up the commit history.

@fzakaria fzakaria force-pushed the fzakaria/nar-kaitai-spec branch from 6a1b5c5 to 22b1c55 Compare October 28, 2025 01:59
@Ericson2314 Ericson2314 enabled auto-merge October 28, 2025 02:38
@Ericson2314 Ericson2314 added this pull request to the merge queue Oct 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2025
@fzakaria
Copy link
Contributor Author

fzakaria commented Oct 28, 2025 •
edited
Loading

Why did the master branch merge queue fail with the pre-commit "relaxed" thing but the PR passed?

@Ericson2314 Ericson2314 added this pull request to the merge queue Oct 28, 2025
@Ericson2314
Copy link
Member

Ericson2314 commented Oct 28, 2025

I don't know. Trying it again.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 28, 2025
@fzakaria
Copy link
Contributor Author

fzakaria commented Oct 28, 2025

want to try again ?

@Ericson2314 Ericson2314 added this pull request to the merge queue Oct 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 29, 2025
@tomberek tomberek added this pull request to the merge queue Oct 29, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 29, 2025
@fzakaria fzakaria force-pushed the fzakaria/nar-kaitai-spec branch from 22b1c55 to 9a7496e Compare October 31, 2025 18:58
@roberth roberth mentioned this pull request Nov 2, 2025
Merged
@fzakaria fzakaria force-pushed the fzakaria/nar-kaitai-spec branch from 9a7496e to 0d5f8de Compare November 2, 2025 16:25
@fzakaria @Ericson2314
Add documentation for NAR spec in kaitai
53b4ea6 
* Add a new flake check
* Add unit tests
* Add Kaitai spec
* Updated documentation
@Ericson2314 Ericson2314 force-pushed the fzakaria/nar-kaitai-spec branch from 0d5f8de to 53b4ea6 Compare November 3, 2025 17:59
@Ericson2314 Ericson2314 enabled auto-merge November 3, 2025 18:01
@Ericson2314 Ericson2314 added this pull request to the merge queue Nov 3, 2025
Merged via the queue into NixOS:master with commit 0586370 Nov 3, 2025
17 checks passed
@roberth roberth mentioned this pull request Nov 3, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ericson2314 Ericson2314 Ericson2314 approved these changes

@edolstra edolstra Awaiting requested review from edolstra edolstra is a code owner

@xokdvium xokdvium Awaiting requested review from xokdvium

+2 more reviewers

@drupol drupol drupol left review comments

@cole-h cole-h cole-h approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

documentation new-cli Relating to the "nix" command

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@fzakaria @Ericson2314 @nixos-discourse @drupol @cole-h @xokdvium