Skip to content

libstore: introduce CanonicalizePathMetadataOptions for canonicalisePathMetaData#15119

Merged
Ericson2314 merged 2 commits intoNixOS:masterfrom
obsidiansystems:canonicalize-pmd-options
Feb 3, 2026
Merged

libstore: introduce CanonicalizePathMetadataOptions for canonicalisePathMetaData#15119
Ericson2314 merged 2 commits intoNixOS:masterfrom
obsidiansystems:canonicalize-pmd-options

Conversation

@amaanq
Copy link
Member

@amaanq amaanq commented Jan 29, 2026

Motivation

This commit refactors the canonicalisePathMetaData function to take an options struct instead of individual parameters with platform-specific #ifdefs.

The struct contains a uidRange field (Unix only) for build user ownership validation, and an ignoredAcls field for ACLs to skip when removing extended attributes.

Context

This PR splits out some work from #15101


Add 👍 to pull requests you find important.

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

@github-actions github-actions bot added new-cli Relating to the "nix" command store Issues and pull requests concerning the Nix store labels Jan 29, 2026
@Ericson2314 Ericson2314 force-pushed the canonicalize-pmd-options branch 4 times, most recently from 12f3f1f to 093b225 Compare January 29, 2026 22:34
@Ericson2314 Ericson2314 enabled auto-merge January 29, 2026 22:34
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

I split the history for posterity's sake

`LocalStore` and `canonicalisePathMetaData` are defined on Windows by
now, so we don't have to gate their usage like this.
@Ericson2314 Ericson2314 force-pushed the canonicalize-pmd-options branch from 093b225 to 37be9be Compare January 30, 2026 00:38
@Ericson2314 Ericson2314 disabled auto-merge January 30, 2026 00:57
Ericson2314 added a commit to obsidiansystems/nix that referenced this pull request Jan 30, 2026
…em meta data

This should not happen now, but instead happen after

- NixOS#15119
- NixOS#15060
- Sergei's upcoming new `Descriptor`-based `SourceAccessor`

I suspect what we'll want to do is expose that source accessor after
all, so we can have some extra methods to get at the underlying file
descriptors. (Or, conversely, maybe this won't be necessary, because enough of the
underlying logic will be factored into `file-descriptor.hh` functions
that the `SourceAccessor` itself will be a small wrapper.)

Either way, at that point we'll not be duplicating stuff here, nor will
be lacking a foundation on Windows, and we can then finish the job.
@Ericson2314 Ericson2314 force-pushed the canonicalize-pmd-options branch from 37be9be to 7400638 Compare January 30, 2026 15:45
Comment on lines +1594 to +1595
// builder UIDs are already dealt with
.uidRange = std::nullopt,
Copy link
Member

Choose a reason for hiding this comment

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

This is not a change in behavior, this is just making the default value (before and after this PR) explicit, and documenting why.

// builder UIDs are already dealt with
.uidRange = std::nullopt,
Copy link
Member

Choose a reason for hiding this comment

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

ditto

…sePathMetaData`

This commit refactors the `canonicalisePathMetaData` function to take an
options struct instead of individual parameters with platform-specific
`#ifdef`s.

The struct contains a `uidRange` field (Unix only) for build user
ownership validation, and an `ignoredAcls` field for ACLs to skip when
removing extended attributes
@Ericson2314 Ericson2314 force-pushed the canonicalize-pmd-options branch from 7400638 to d09f03d Compare January 30, 2026 16:06
@Ericson2314 Ericson2314 added this pull request to the merge queue Feb 3, 2026
Merged via the queue into NixOS:master with commit 0e2fc2a Feb 3, 2026
14 checks passed
@Ericson2314 Ericson2314 deleted the canonicalize-pmd-options branch February 3, 2026 20:45
Ericson2314 added a commit to obsidiansystems/nix that referenced this pull request Feb 5, 2026
…em meta data

This should not happen now, but instead happen after

- NixOS#15119
- NixOS#15060
- Sergei's upcoming new `Descriptor`-based `SourceAccessor`

I suspect what we'll want to do is expose that source accessor after
all, so we can have some extra methods to get at the underlying file
descriptors. (Or, conversely, maybe this won't be necessary, because enough of the
underlying logic will be factored into `file-descriptor.hh` functions
that the `SourceAccessor` itself will be a small wrapper.)

Either way, at that point we'll not be duplicating stuff here, nor will
be lacking a foundation on Windows, and we can then finish the job.
Ericson2314 added a commit to obsidiansystems/nix that referenced this pull request Feb 5, 2026
…em meta data

This should not happen now, but instead happen after

- NixOS#15119
- NixOS#15060
- Sergei's upcoming new `Descriptor`-based `SourceAccessor`

I suspect what we'll want to do is expose that source accessor after
all, so we can have some extra methods to get at the underlying file
descriptors. (Or, conversely, maybe this won't be necessary, because enough of the
underlying logic will be factored into `file-descriptor.hh` functions
that the `SourceAccessor` itself will be a small wrapper.)

Either way, at that point we'll not be duplicating stuff here, nor will
be lacking a foundation on Windows, and we can then finish the job.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-cli Relating to the "nix" command store Issues and pull requests concerning the Nix store

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants