OstreeMutableTree: Add parent pointer and fix bug #1655
Closed
Conversation
This file contains 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
This implements a TODO item from `ostree_mutable_tree_get_contents_checksum`. We now no-longer invalidate the dirtree contents checksum at `get_contents_checksum` time - we invalidate it when the mtree is modified. This is implemented by keeping a pointer to the parent directory in each `OstreeMutableTree`. This gives us stronger invariants on `contents_checksum`. For even stronger guarantees about invariants we could make `ostree_repo_write_mtree` or similar a member of `OstreeMutableTree` and remove `ostree_mutable_tree_set_metadata_checksum`. I think I've fixed a bug here too. We now invalidate parent's contents checksum when our metadata checksum changes, whereas we didn't before.
…changes This bug has existed before the previous commit, but thanks to the previous commit it is now easy to fix.
3 tasks
jlebon
reviewed
Jun 27, 2018
src/libostree/ostree-mutable-tree.c
Outdated
| @@ -146,7 +149,7 @@ invalidate_contents_checksum (OstreeMutableTree *self) | |||
| break; | |||
|
|
|||
| g_free (self->contents_checksum); | |||
There was a problem hiding this comment.
You can drop this line too, that's the beauty of g_clear_pointer :)
| /* If a child tree's metadata changes the parent tree's contents needs to be | ||
| * recalculated */ | ||
| glnx_unref_object OstreeMutableTree *subdir = NULL; | ||
| g_assert (ostree_mutable_tree_ensure_dir (tree, "subdir", &subdir, &error)); |
There was a problem hiding this comment.
If we're doing a g_assert and we're not gonna check that error is still NULL, we might as well pass NULL here and drop the local variable.
|
Awesome, thanks a lot for this! |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 28, 2018
This implements a TODO item from `ostree_mutable_tree_get_contents_checksum`. We now no-longer invalidate the dirtree contents checksum at `get_contents_checksum` time - we invalidate it when the mtree is modified. This is implemented by keeping a pointer to the parent directory in each `OstreeMutableTree`. This gives us stronger invariants on `contents_checksum`. For even stronger guarantees about invariants we could make `ostree_repo_write_mtree` or similar a member of `OstreeMutableTree` and remove `ostree_mutable_tree_set_metadata_checksum`. I think I've fixed a bug here too. We now invalidate parent's contents checksum when our metadata checksum changes, whereas we didn't before. Closes: #1655 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 28, 2018
…changes This bug has existed before the previous commit, but thanks to the previous commit it is now easy to fix. Closes: #1655 Approved by: cgwalters
|
💥 Test timed out |
|
@rh-atomic-bot retry |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 29, 2018
This implements a TODO item from `ostree_mutable_tree_get_contents_checksum`. We now no-longer invalidate the dirtree contents checksum at `get_contents_checksum` time - we invalidate it when the mtree is modified. This is implemented by keeping a pointer to the parent directory in each `OstreeMutableTree`. This gives us stronger invariants on `contents_checksum`. For even stronger guarantees about invariants we could make `ostree_repo_write_mtree` or similar a member of `OstreeMutableTree` and remove `ostree_mutable_tree_set_metadata_checksum`. I think I've fixed a bug here too. We now invalidate parent's contents checksum when our metadata checksum changes, whereas we didn't before. Closes: #1655 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 29, 2018
…changes This bug has existed before the previous commit, but thanks to the previous commit it is now easy to fix. Closes: #1655 Approved by: cgwalters
|
💔 Test failed - status-atomicjenkins |
cgwalters
added a commit
to cgwalters/ostree
that referenced
this pull request
Jun 29, 2018
Noticed as part of a random failure in this PR: ostreedev#1655 that we weren't actually testing the version of ostree we built in git. Probably we should be generating RPMs but...later.
|
OK right, this is failing because of #1662 and... not sure what happened with the |
|
We can re-queue this one now assuming the other one lands. @rh-atomic-bot retry |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 29, 2018
…changes This bug has existed before the previous commit, but thanks to the previous commit it is now easy to fix. Closes: #1655 Approved by: cgwalters
|
☀️ Test successful - status-atomicjenkins |
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.
This is preparatory work for #1643. The commits were originally on #1643 and I've added a fixup to address some of the comments from there. I split this out as it's own PR as requested by @jlebon.
This implements a TODO item from
ostree_mutable_tree_get_contents_checksum. We now no-longer invalidatethe dirtree contents checksum at
get_contents_checksumtime - weinvalidate it when the mtree is modified. This is implemented by keeping
a pointer to the parent directory in each
OstreeMutableTree. This givesus stronger invariants on
contents_checksum.For even stronger guarantees about invariants we could make
ostree_repo_write_mtreeor similar a member ofOstreeMutableTreeandremove
ostree_mutable_tree_set_metadata_checksum.I've fixed a bug here too. We now invalidate parent's contents
checksum when our metadata checksum changes, whereas we didn't before.