[PATCH v5 00/11] Deprecate .git/info/grafts

[Johannes Schindelin <johannes.schindelin@gmx.de>]

It is fragile, as there is no way for the revision machinery to say "but
now I want to traverse the graph ignoring the graft file" e.g. when
pushing commits to a remote repository (which, as a consequence, can
miss commits).

And we already have a better solution with `git replace --graft <comit>
[<parent>...]`.

Changes since v4:

- Fixed resource leaks (missing free() and strbuf_release() calls when
  returning an error).

- Avoided error_errno() mistakenly using close()'s errno.

- Actually close the handle to the graft file after converting it.

- Changed the description of Documentation/technical/shallow to not even
  bother to describe how it might be construed to be similar to replace
  refs.


Johannes Schindelin (11):
  argv_array: offer to split a string by whitespace
  commit: Let the callback of for_each_mergetag return on error
  replace: avoid using die() to indicate a bug
  replace: "libify" create_graft() and callees
  replace: introduce --convert-graft-file
  Add a test for `git replace --convert-graft-file`
  Deprecate support for .git/info/grafts
  filter-branch: stop suggesting to use grafts
  technical/shallow: stop referring to grafts
  technical/shallow: describe why shallow cannot use replace refs
  Remove obsolete script to convert grafts to replace refs

 Documentation/git-filter-branch.txt       |   2 +-
 Documentation/git-replace.txt             |  11 +-
 Documentation/technical/shallow.txt       |  20 +-
 advice.c                                  |   2 +
 advice.h                                  |   1 +
 argv-array.c                              |  20 ++
 argv-array.h                              |   2 +
 builtin/replace.c                         | 218 +++++++++++++++-------
 commit.c                                  |  18 +-
 commit.h                                  |   4 +-
 contrib/convert-grafts-to-replace-refs.sh |  28 ---
 log-tree.c                                |  13 +-
 t/t6001-rev-list-graft.sh                 |   9 +
 t/t6050-replace.sh                        |  20 ++
 14 files changed, 253 insertions(+), 115 deletions(-)
 delete mode 100755 contrib/convert-grafts-to-replace-refs.sh


base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
Published-As: https://github.com/dscho/git/releases/tag/deprecate-grafts-v5
Fetch-It-Via: git fetch https://github.com/dscho/git deprecate-grafts-v5

Interdiff vs v4:
 diff --git a/Documentation/technical/shallow.txt b/Documentation/technical/shallow.txt
 index cb79181c2bb..01dedfe9ffe 100644
 --- a/Documentation/technical/shallow.txt
 +++ b/Documentation/technical/shallow.txt
 @@ -8,18 +8,10 @@ repo, and therefore grafts are introduced pretending that
  these commits have no parents.
  *********************************************************
  
 -The basic idea is to write the SHA-1s of shallow commits into
 -$GIT_DIR/shallow, and handle its contents similar to replace
 -refs (with the difference that shallow does not actually
 -create those replace refs) and very much like the deprecated
 -graft file (with the difference that shallow commits will
 -always have their parents grafted away, not replaced by
 -different parents).
 -
 -This information is stored in a special-purpose file because the
 -user should not touch that file at all (even throughout
 -development of the shallow clone, it was never manually
 -edited!).
 +$GIT_DIR/shallow lists commit object names and tells Git to
 +pretend as if they are root commits (e.g. "git log" traversal
 +stops after showing them; "git fsck" does not complain saying
 +the commits listed on their "parent" lines do not exist).
  
  Each line contains exactly one SHA-1. When read, a commit_graft
  will be constructed, which has nr_parent < 0 to make it easier
 diff --git a/builtin/replace.c b/builtin/replace.c
 index 6b3e0301e90..acd30e3d824 100644
 --- a/builtin/replace.c
 +++ b/builtin/replace.c
 @@ -175,8 +175,10 @@ static int replace_object_oid(const char *object_ref,
  			     object_ref, type_name(obj_type),
  			     replace_ref, type_name(repl_type));
  
 -	if (check_ref_valid(object, &prev, &ref, force))
 +	if (check_ref_valid(object, &prev, &ref, force)) {
 +		strbuf_release(&ref);
  		return -1;
 +	}
  
  	transaction = ref_transaction_begin(&err);
  	if (!transaction ||
 @@ -264,9 +266,10 @@ static int import_object(struct object_id *oid, enum object_type type,
  		}
  
  		if (strbuf_read(&result, cmd.out, 41) < 0) {
 +			error_errno("unable to read from mktree");
  			close(fd);
  			close(cmd.out);
 -			return error_errno("unable to read from mktree");
 +			return -1;
  		}
  		close(cmd.out);
  
 @@ -285,8 +288,9 @@ static int import_object(struct object_id *oid, enum object_type type,
  		int flags = HASH_FORMAT_CHECK | HASH_WRITE_OBJECT;
  
  		if (fstat(fd, &st) < 0) {
 +			error_errno("unable to fstat %s", filename);
  			close(fd);
 -			return error_errno("unable to fstat %s", filename);
 +			return -1;
  		}
  		if (index_fd(oid, fd, &st, type, NULL, flags) < 0)
  			return error("unable to write object to database");
 @@ -302,7 +306,7 @@ static int import_object(struct object_id *oid, enum object_type type,
  
  static int edit_and_replace(const char *object_ref, int force, int raw)
  {
 -	char *tmpfile = git_pathdup("REPLACE_EDITOBJ");
 +	char *tmpfile;
  	enum object_type type;
  	struct object_id old_oid, new_oid, prev;
  	struct strbuf ref = STRBUF_INIT;
 @@ -315,17 +319,20 @@ static int edit_and_replace(const char *object_ref, int force, int raw)
  		return error("unable to get object type for %s",
  			     oid_to_hex(&old_oid));
  
 -	if (check_ref_valid(&old_oid, &prev, &ref, force))
 +	if (check_ref_valid(&old_oid, &prev, &ref, force)) {
 +		strbuf_release(&ref);
  		return -1;
 +	}
  	strbuf_release(&ref);
  
 -	if (export_object(&old_oid, type, raw, tmpfile))
 -		return -1;
 -	if (launch_editor(tmpfile, NULL, NULL) < 0)
 -		return error("editing object file failed");
 -	if (import_object(&new_oid, type, raw, tmpfile))
 +	tmpfile = git_pathdup("REPLACE_EDITOBJ");
 +	if (export_object(&old_oid, type, raw, tmpfile) ||
 +	    (launch_editor(tmpfile, NULL, NULL) < 0 &&
 +	     error("editing object file failed")) ||
 +	    import_object(&new_oid, type, raw, tmpfile)) {
 +		free(tmpfile);
  		return -1;
 -
 +	}
  	free(tmpfile);
  
  	if (!oidcmp(&old_oid, &new_oid))
 @@ -351,11 +358,15 @@ static int replace_parents(struct strbuf *buf, int argc, const char **argv)
  	/* prepare new parents */
  	for (i = 0; i < argc; i++) {
  		struct object_id oid;
 -		if (get_oid(argv[i], &oid) < 0)
 +		if (get_oid(argv[i], &oid) < 0) {
 +			strbuf_release(&new_parents);
  			return error(_("Not a valid object name: '%s'"),
  				     argv[i]);
 -		if (!lookup_commit_reference(&oid))
 +		}
 +		if (!lookup_commit_reference(&oid)) {
 +			strbuf_release(&new_parents);
  			return error(_("could not parse %s"), argv[i]);
 +		}
  		strbuf_addf(&new_parents, "parent %s\n", oid_to_hex(&oid));
  	}
  
 @@ -432,20 +443,26 @@ static int create_graft(int argc, const char **argv, int force)
  	strbuf_add(&buf, buffer, size);
  	unuse_commit_buffer(commit, buffer);
  
 -	if (replace_parents(&buf, argc - 1, &argv[1]) < 0)
 +	if (replace_parents(&buf, argc - 1, &argv[1]) < 0) {
 +		strbuf_release(&buf);
  		return -1;
 +	}
  
  	if (remove_signature(&buf)) {
  		warning(_("the original commit '%s' has a gpg signature."), old_ref);
  		warning(_("the signature will be removed in the replacement commit!"));
  	}
  
 -	if (check_mergetags(commit, argc, argv))
 +	if (check_mergetags(commit, argc, argv)) {
 +		strbuf_release(&buf);
  		return -1;
 +	}
  
 -	if (write_object_file(buf.buf, buf.len, commit_type, &new_oid))
 +	if (write_object_file(buf.buf, buf.len, commit_type, &new_oid)) {
 +		strbuf_release(&buf);
  		return error(_("could not write replacement commit for: '%s'"),
  			     old_ref);
 +	}
  
  	strbuf_release(&buf);
  
 @@ -474,9 +491,9 @@ static int convert_graft_file(int force)
  			strbuf_addf(&err, "\n\t%s", buf.buf);
  		argv_array_clear(&args);
  	}
 +	fclose(fp);
  
  	strbuf_release(&buf);
 -	argv_array_clear(&args);
  
  	if (!err.len)
  		return unlink_or_warn(graft_file);
-- 
2.17.0.windows.1.33.gfcbb1fa0445