@matheusbsbs / matheus-bagliano / commits / 2f00c355cb

Fork 0

commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flag

Since 7c5c9b9c57 (commit-graph: error out on invalid commit oids in 'write --stdin-commits', 2019-08-05), the commit-graph builtin dies on receiving non-commit OIDs as input to '--stdin-commits'. This behavior can be cumbersome to work around in, say, the case of piping 'git for-each-ref' to 'git commit-graph write --stdin-commits' if the caller does not want to cull out non-commits themselves. In this situation, it would be ideal if 'git commit-graph write' wrote the graph containing the inputs that did pertain to commits, and silently ignored the remainder of the input. Some options have been proposed to the effect of '--[no-]check-oids' which would allow callers to have the commit-graph builtin do just that. After some discussion, it is difficult to imagine a caller who wouldn't want to pass '--no-check-oids', suggesting that we should get rid of the behavior of complaining about non-commit inputs altogether. If callers do wish to retain this behavior, they can easily work around this change by doing the following: git for-each-ref --format='%(objectname) %(objecttype) %(*objecttype)' | awk ' !/commit/ { print "not-a-commit:"$1 } /commit/ { print $1 } ' | git commit-graph write --stdin-commits To make it so that valid OIDs that refer to non-existent objects are indeed an error after loosening the error handling, perform an extra lookup to make sure that object indeed exists before sending it to the commit-graph internals. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed May 13, 2020 at 15:59 UTC 2f00c355cb79ee86bddc9f2fef91ac380a6023fc

5 files changed +24 -18

Documentation/git-commit-graph.txt

+4 -2

@@ -47,8 +47,10 @@ with `--stdin-commits` or `--reachable`.)

47 +

48 With the `--stdin-commits` option, generate the new commit graph by

49 walking commits starting at the commits specified in stdin as a list

50 -of OIDs in hex, one OID per line. (Cannot be combined with

51 -`--stdin-packs` or `--reachable`.)

50 +of OIDs in hex, one OID per line. OIDs that resolve to non-commits

51 +(either directly, or by peeling tags) are silently ignored. OIDs that

52 +are malformed, or do not exist generate an error. (Cannot be combined

53 +with `--stdin-packs` or `--reachable`.)

54 +

55 With the `--reachable` option, generate the new commit graph by walking

56 commits starting at all refs. (Cannot be combined with `--stdin-commits`

builtin/commit-graph.c

+8 -7

@@ -7,6 +7,7 @@

7 #include "commit-graph.h"

8 #include "object-store.h"

9 #include "progress.h"

10 +#include "tag.h"

12 static char const * const builtin_commit_graph_usage[] = {

13 N_("git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]"),

@@ -142,18 +143,19 @@ static int write_option_parse_split(const struct option *opt, const char *arg,

143 static int read_one_commit(struct oidset *commits, struct progress *progress,

144 const char *hash)

145 {

145 - struct commit *result;

146 + struct object *result;

147 struct object_id oid;

148 const char *end;

149

150 if (parse_oid_hex(hash, &oid, &end))

151 return error(_("unexpected non-hex object ID: %s"), hash);

152

152 - result = lookup_commit_reference_gently(the_repository, &oid, 1);

153 - if (result)

154 - oidset_insert(commits, &result->object.oid);

155 - else

156 - return error(_("invalid commit object id: %s"), hash);

153 + result = deref_tag(the_repository, parse_object(the_repository, &oid),

154 + NULL, 0);

155 + if (!result)

156 + return error(_("invalid object: %s"), hash);

157 + else if (object_as_type(the_repository, result, OBJ_COMMIT, 1))

158 + oidset_insert(commits, &result->oid);

159

160 display_progress(progress, oidset_size(commits));

161

@@ -238,7 +240,6 @@ static int graph_write(int argc, const char **argv)

240 strbuf_detach(&buf, NULL));

241 } else if (opts.stdin_commits) {

242 oidset_init(&commits, 0);

241 - flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;

243 if (opts.progress)

244 progress = start_delayed_progress(

245 _("Collecting commits from input"), 0);

commit-graph.c

-2

@@ -880,7 +880,6 @@ struct write_commit_graph_context {

880 unsigned append:1,

881 report_progress:1,

882 split:1,

883 - check_oids:1,

883 changed_paths:1,

884 order_by_pack:1;

885

@@ -2002,7 +2001,6 @@ int write_commit_graph(struct object_directory *odb,

2001 ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;

2002 ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;

2003 ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;

2005 - ctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;

2004 ctx->split_opts = split_opts;

2005 ctx->changed_paths = flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS ? 1 : 0;

2006 ctx->total_bloom_filter_data_size = 0;

commit-graph.h

+1 -3

@@ -91,9 +91,7 @@ enum commit_graph_write_flags {

91 COMMIT_GRAPH_WRITE_APPEND = (1 << 0),

92 COMMIT_GRAPH_WRITE_PROGRESS = (1 << 1),

93 COMMIT_GRAPH_WRITE_SPLIT = (1 << 2),

94 - /* Make sure that each OID in the input is a valid commit OID. */

95 - COMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3),

96 - COMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 4),

94 + COMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 3),

95 };

97 enum commit_graph_split_flags {

t/t5318-commit-graph.sh

+11 -4

@@ -84,11 +84,18 @@ graph_read_expect() {

85 test_expect_success 'exit with correct error on bad input to --stdin-commits' '

86 cd "$TRASH_DIRECTORY/full" &&

87 - echo HEAD | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&

87 + # invalid, non-hex OID

88 + echo HEAD >in &&

89 + test_expect_code 1 git commit-graph write --stdin-commits <in 2>stderr &&

90 test_i18ngrep "unexpected non-hex object ID: HEAD" stderr &&

89 - # valid tree OID, but not a commit OID

90 - git rev-parse HEAD^{tree} | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&

91 - test_i18ngrep "invalid commit object id" stderr

91 + # non-existent OID

92 + echo $ZERO_OID >in &&

93 + test_expect_code 1 git commit-graph write --stdin-commits <in 2>stderr &&

94 + test_i18ngrep "invalid object" stderr &&

95 + # valid commit and tree OID

96 + git rev-parse HEAD HEAD^{tree} >in &&

97 + git commit-graph write --stdin-commits <in &&

98 + graph_read_expect 3

99 '

100

101 test_expect_success 'write graph' '