tree: accept null ids in existing trees when updating #4727
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
When we add entries to a treebuilder we validate them. But we validate even those that we're adding because they exist in the base tree. This disables using the normal mechanisms on these trees, even to fix them. Keep track of whether the entry we're appending comes from an existing tree and bypass the name and id validation if it's from existing data.
|
This would be one for backporting into a maintenance release. |
pks-t
reviewed
Jul 19, 2018
src/tree.c
Outdated
| @@ -447,15 +447,16 @@ static int append_entry( | |||
| git_treebuilder *bld, | |||
| const char *filename, | |||
| const git_oid *id, | |||
| git_filemode_t filemode) | |||
| git_filemode_t filemode, | |||
| bool from_tree) | |||
There was a problem hiding this comment.
I was misreading this commit the whole time, as I first misinterpreted the from_tree parameter to mean the exact opposite thing. I think this would be a bit easier to reason about if this was instead called validate, with the value passed by callers being the inverse from what it is now.
pks-t
reviewed
Jul 19, 2018
| @@ -284,3 +284,18 @@ void test_object_tree_update__add_conflict2(void) | |||
|
|
|||
| cl_git_fail(git_tree_create_updated(&tree_updater_id, g_repo, NULL, 2, updates)); | |||
| } | |||
|
|
|||
| void test_object_tree_update__remove_invalid_submodule(void) | |||
There was a problem hiding this comment.
Could you please add a small comment detailing what is so special about the tree 396c7f1? It's rather hard to reason about this test otherwise :)
|
Updated from @pks-t 's feedback |
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.
When we add entries to a treebuilder we validate them. But we validate even
those that we're adding because they exist in the base tree. This disables
using the normal mechanisms on these trees, even to fix them.
Keep track of whether the entry we're appending comes from an existing tree and
bypass the name and id validation if it's from existing data.