git/t/t5318-commit-graph.sh, branch v2.25.2

Merge branch 'ds/commit-graph-delay-gen-progress'

2019-12-10T21:11:43Z

One kind of progress messages were always given during commit-graph generation, instead of following the "if it takes more than two seconds, show progress" pattern, which has been corrected. * ds/commit-graph-delay-gen-progress: commit-graph: use start_delayed_progress() progress: create GIT_PROGRESS_DELAY

Merge branch 'ds/test-read-graph'

2019-12-01T17:04:39Z

Dev support for commit-graph feature. * ds/test-read-graph: test-tool: use 'read-graph' helper

progress: create GIT_PROGRESS_DELAY

2019-11-27T01:57:10Z

The start_delayed_progress() method is a preferred way to show optional progress to users as it ignores steps that take less than two seconds. However, this makes testing unreliable as tests expect to be very fast. In addition, users may want to decrease or increase this time interval depending on their preferences for terminal noise. Create the GIT_PROGRESS_DELAY environment variable to control the delay set during start_delayed_progress(). Set the value in some tests to guarantee their output remains consistent. Helped-by: Jeff King Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano

test-tool: use 'read-graph' helper

2019-11-13T02:14:16Z

The 'git commit-graph read' subcommand is used in test scripts to check that the commit-graph contents match the expected data. Mostly, this helps check the header information and the list of chunks. Users do not need this information, so move the functionality to a test helper. Reported-by: Bryan Turner Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano

commit, tag: don't set parsed bit for parse failures

2019-10-28T05:04:49Z

If we can't parse a commit, then parse_commit() will return an error code. But it _also_ sets the "parsed" flag, which tells us not to bother trying to re-parse the object. That means that subsequent parses have no idea that the information in the struct may be bogus. I.e., doing this: parse_commit(commit); ... if (parse_commit(commit) < 0) die("commit is broken"); will never trigger the die(). The second parse_commit() will see the "parsed" flag and quietly return success. There are two obvious ways to fix this: 1. Stop setting "parsed" until we've successfully parsed. 2. Keep a second "corrupt" flag to indicate that we saw an error (and when the parsed flag is set, return 0/-1 depending on the corrupt flag). This patch does option 1. The obvious downside versus option 2 is that we might continually re-parse a broken object. But in practice, corruption like this is rare, and we typically die() or return an error in the caller. So it's OK not to worry about optimizing for corruption. And it's much simpler: we don't need to use an extra bit in the object struct, and callers which check the "parsed" flag don't need to learn about the corrupt bit, too. There's no new test here, because this case is already covered in t5318. Note that we do need to update the expected message there, because we now detect the problem in the return from "parse_commit()", and not with a separate check for a NULL tree. In fact, we can now ditch that explicit tree check entirely, as we're covered robustly by this change (and the previous recent change to treat a NULL tree as a parse error). We'll also give tags the same treatment. I don't know offhand of any cases where the problem can be triggered (it implies somebody ignoring a parse error earlier in the process), but consistently returning an error should cause the least surprise. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'tb/commit-graph-harden'

2019-10-07T02:32:58Z

The code to parse and use the commit-graph file has been made more robust against corrupted input. * tb/commit-graph-harden: commit-graph.c: handle corrupt/missing trees commit-graph.c: handle commit parsing errors t/t5318: introduce failing 'git commit-graph write' tests

commit-graph: add --[no-]progress to write and verify

2019-09-18T21:23:09Z

Add --[no-]progress to git commit-graph write and verify. The progress feature was introduced in 7b0f229 ("commit-graph write: add progress output", 2018-09-17) but the ability to opt-out was overlooked. Signed-off-by: Garima Singh Signed-off-by: Junio C Hamano

commit-graph.c: handle corrupt/missing trees

2019-09-09T17:55:59Z

Apply similar treatment as in the previous commit to handle an unchecked call to 'get_commit_tree_oid()'. Previously, a NULL return value from this function would be immediately dereferenced with '->hash', and then cause a segfault. Before dereferencing to access the 'hash' member, check the return value of 'get_commit_tree_oid()' to make sure that it is not NULL. To make this check correct, a related change is also needed in 'commit.c', which is to check the return value of 'get_commit_tree' before taking its address. If 'get_commit_tree' returns NULL, we encounter an undefined behavior when taking the address of the return value of 'get_commit_tree' and then taking '->object.oid'. (On my system, this is memory address 0x8, which is obviously wrong). Fix this by making sure that 'get_commit_tree' returns something non-NULL before digging through a structure that is not there, thus preventing a segfault down the line in the commit graph code. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano

commit-graph.c: handle commit parsing errors

2019-09-09T17:55:58Z

To write a commit graph chunk, 'write_graph_chunk_data()' takes a list of commits to write and parses each one before writing the necessary data, and continuing on to the next commit in the list. Since the majority of these commits are not parsed ahead of time (an exception is made for the *last* commit in the list, which is parsed early within 'copy_oids_to_commits'), it is possible that calling 'parse_commit_no_graph()' on them may return an error. Failing to catch these errors before de-referencing later calls can result in a undefined memory access and a SIGSEGV. One such example of this is 'get_commit_tree_oid()', which expects a parsed object as its input (in this case, the commit-graph code passes '*list'). If '*list' causes a parse error, the subsequent call will fail. Prevent such an issue by checking the return value of 'parse_commit_no_graph()' to avoid passing an unparsed object to a function which expects a parsed object, thus preventing a segfault. It is worth noting that this fix is really skirting around the issue in object.c's 'parse_object()', which makes it difficult to tell how corrupt an object is without digging into it. Presumably one could change the meaning of 'parse_object' returns, but this would require adjusting each callsite accordingly. Instead of that, add an additional check to the object parsed. Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano

t/t5318: introduce failing 'git commit-graph write' tests

2019-09-09T17:55:53Z

When invoking 'git commit-graph' in a corrupt repository, one can cause a segfault when ancestral commits are corrupt in one way or another. This is due to two function calls in the 'commit-graph.c' code that may return NULL, but are not checked for NULL-ness before dereferencing. Before fixing the bug, introduce two failing tests that demonstrate the problem. The first test corrupts an ancestral commit's parent to point to a non-existent object. The second test instead corrupts an ancestral tree by removing the 'tree' information entirely from the commit. Both of these cases cause segfaults, each at different lines. Signed-off-by: Taylor Blau Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano