Various repository fixes #146
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cgwalters
approved these changes
Jun 17, 2025
crates/composefs/src/util.rs
Outdated
| format!("{}{}", prefix, rand_string) | ||
| } | ||
|
|
||
| pub fn replace_symlinkat( |
There was a problem hiding this comment.
I think unit tests for this would be pretty easy to add too
alexlarsson
force-pushed
the
repository-fixes
branch
from
08611b7 to
362631d
Compare
cgwalters
reviewed
Jun 17, 2025
| let category_fd = self.openat(category, OFlags::RDONLY | OFlags::DIRECTORY)?; | ||
| let category_fd = match filter_errno( | ||
| self.openat(category, OFlags::RDONLY | OFlags::DIRECTORY), | ||
| Errno::NOENT, |
There was a problem hiding this comment.
BTW I maintain https://docs.rs/cap-std-ext/latest/cap_std_ext/dirext/trait.CapStdExtDirExt.html#tymethod.open_dir_optional
We had some previous debates about using cap-std here. I (obviously) like it a lot.
filter_errno makes sense as is but it'd probably be good to have a higher level openat_optional wrapper per above.
crates/composefs/src/repository.rs
Outdated
| Errno::NOENT, | ||
| ) | ||
| .context("Opening {category} dir in repository")? | ||
| { |
There was a problem hiding this comment.
This is begging for a
let Some(category_fd) = ...? else {
return;
}| } | ||
| } | ||
|
|
||
| Err(Errno::EXIST) |
There was a problem hiding this comment.
That could probably use some context...
allisonkarlitskaya
force-pushed
the
repository-fixes
branch
from
362631d to
0f8d2eb
Compare
|
What happened with the commit description? |
|
Uh. There's very clearly some rebase fail here, sorry :( |
For example, if there has been no streams created, we don't want to fail with some random ENOENT. Signed-off-by: Alexander Larsson <[email protected]>
Naming a new version of an image the same as the old version is pretty standard, and this is currently giving EEXIST errors. We fix this by doing an atomic symlink + rename operation to replace the symlink if it doesn't already have the expected value. Drop our existing Repository::ensure_symlink() function and use the new function for that as well. Co-authored-by: Alexander Larsson <[email protected]> Signed-off-by: Alexander Larsson <[email protected]> Signed-off-by: Allison Karlitskaya <[email protected]>
This adds error context when opening images and streams. Without this it is kinda hard to figure out what ENOENT really means. Signed-off-by: Alexander Larsson <[email protected]>
allisonkarlitskaya
force-pushed
the
repository-fixes
branch
from
0f8d2eb to
fff41b1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These were broken out from: #144