Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

commit-graph: writing missing parents is a BUG#97

Merged
derrickstolee merged 1 commit intomicrosoft:vfs-2.20.1microsoft/git:vfs-2.20.1from
derrickstolee:commit-graph-bugderrickstolee/git:commit-graph-bugCopy head branch name to clipboard
Dec 19, 2018
Merged

commit-graph: writing missing parents is a BUG#97
derrickstolee merged 1 commit intomicrosoft:vfs-2.20.1microsoft/git:vfs-2.20.1from
derrickstolee:commit-graph-bugderrickstolee/git:commit-graph-bugCopy head branch name to clipboard

Conversation

@derrickstolee
Copy link

When writing a commit-graph, we write GRAPH_MISSING_PARENT if the
parent's object id does not appear in the list of commits to be
written into the commit-graph. This was done as the initial design
allowed commits to have missing parents, but the final version
requires the commit-graph to be closed under reachability. Thus,
this GRAPH_MISSING_PARENT value should never be written.

However, there are reasons why it could be written! These range
from a bug in the reachable-closure code to a memory error causing
the binary search into the list of object ids to fail. In either
case, we should fail fast and avoid writing the commit-graph file
with bad data.

@kewillford
Copy link
Member

There were a couple of other places that GRAPH_PARENT_MISSING was being used.

	if (count_distinct >= GRAPH_PARENT_MISSING)
		die(_("the commit graph format cannot write %d commits"), count_distinct);
	if (commits.nr >= GRAPH_PARENT_MISSING)
		die(_("too many commits to write graph"));

Should GRAPH_PARENT_MISSING be renamed to something like MAX_NUMBER_OF_COMMITS now?

Copy link
Member

@kewillford kewillford left a comment

Choose a reason for hiding this comment

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

Approve with suggestion.

When writing a commit-graph, we write GRAPH_MISSING_PARENT if the
parent's object id does not appear in the list of commits to be
written into the commit-graph. This was done as the initial design
allowed commits to have missing parents, but the final version
requires the commit-graph to be closed under reachability. Thus,
this GRAPH_MISSING_PARENT value should never be written.

However, there are reasons why it could be written! These range
from a bug in the reachable-closure code to a memory error causing
the binary search into the list of object ids to fail. In either
case, we should fail fast and avoid writing the commit-graph file
with bad data.

Remove the GRAPH_MISSING_PARENT constant in favor of the constant
GRAPH_EDGE_LAST_MASK, which has the same value.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.