Skip to content

Introduce CanonPath::fromFilename#15333

Merged
Ericson2314 merged 1 commit intomasterfrom
canon-path-from-filename
Feb 25, 2026
Merged

Introduce CanonPath::fromFilename#15333
Ericson2314 merged 1 commit intomasterfrom
canon-path-from-filename

Conversation

@xokdvium
Copy link
Contributor

Motivation

See docs for what it is. Use in DerivationBuilderImpl::writeBuilderFile and deletePath. Also adds O_DIRECTORY via openDirectory in deletePath.

Context


Add 👍 to pull requests you find important.

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

See docs for what it is. Use in DerivationBuilderImpl::writeBuilderFile
and deletePath. Also adds O_DIRECTORY via openDirectory in deletePath.

Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
Comment on lines +29 to +30
/* Use existing canonicalisation logic for CanonPath to check that the segment
is already a valid filename. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to me that doing it this way is safer and has a lower chance of getting out of sync.

Copy link
Member

Choose a reason for hiding this comment

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

OK that works for me

assert(parentDirPath != path);

AutoCloseFD dirfd = toDescriptor(open(path.parent_path().string().c_str(), O_RDONLY));
AutoCloseFD dirfd = openDirectory(parentDirPath);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we can trivially get rid of the .string() churn already.

@Ericson2314 Ericson2314 added this pull request to the merge queue Feb 25, 2026
Merged via the queue into master with commit a2d1346 Feb 25, 2026
18 checks passed
@Ericson2314 Ericson2314 deleted the canon-path-from-filename branch February 25, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants