Skip to content

tree: accept null ids in existing trees when updating#4727

Merged
ethomson merged 2 commits intomasterfrom
cmn/null-oid-existing-tree
Aug 26, 2018
Merged

tree: accept null ids in existing trees when updating#4727
ethomson merged 2 commits intomasterfrom
cmn/null-oid-existing-tree

Conversation

@carlosmn
Copy link
Copy Markdown
Member

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.

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.
@carlosmn
Copy link
Copy Markdown
Member Author

This would be one for backporting into a maintenance release.

Comment thread src/tree.c Outdated
const git_oid *id,
git_filemode_t filemode)
git_filemode_t filemode,
bool from_tree)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

cl_git_fail(git_tree_create_updated(&tree_updater_id, g_repo, NULL, 2, updates));
}

void test_object_tree_update__remove_invalid_submodule(void)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 :)

@carlosmn
Copy link
Copy Markdown
Member Author

Updated from @pks-t 's feedback

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.

3 participants