Re: [PATCH v6 2/7] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs

[Michael Haggerty <mhagger@alum.mit.edu>]

On 06/29/2015 10:17 PM, David Turner wrote:
> Instead of directly writing to and reading from files in
> $GIT_DIR, use ref API to interact with CHERRY_PICK_HEAD
> and REVERT_HEAD.
> 
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  branch.c                         |  4 ++--
>  builtin/commit.c                 |  6 +++---
>  builtin/merge.c                  |  2 +-
>  contrib/completion/git-prompt.sh |  4 ++--
>  git-gui/lib/commit.tcl           |  2 +-
>  sequencer.c                      | 39 ++++++++++++++++++++-------------------
>  t/t7509-commit.sh                |  4 ++--
>  wt-status.c                      |  6 ++----
>  8 files changed, 33 insertions(+), 34 deletions(-)
> 
> [...]
> diff --git a/sequencer.c b/sequencer.c
> index f8421a8..44c43e5 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -158,21 +158,22 @@ static void free_message(struct commit *commit, struct commit_message *msg)
>  	unuse_commit_buffer(commit, msg->message);
>  }
>  
> -static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)
> +static void write_cherry_pick_head(struct commit *commit, const char *ref)
>  {
> -	const char *filename;
> -	int fd;
> -	struct strbuf buf = STRBUF_INIT;
> +	struct strbuf err = STRBUF_INIT;
> +	void *transaction;
>  
> -	strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1));
> +	transaction = ref_transaction_begin(&err);
> +	if (!transaction)
> +		die(_("Could not create transaction: %s"), err.buf);
>  
> -	filename = git_path("%s", pseudoref);
> -	fd = open(filename, O_WRONLY | O_CREAT, 0666);
> -	if (fd < 0)
> -		die_errno(_("Could not open '%s' for writing"), filename);
> -	if (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))
> -		die_errno(_("Could not write to '%s'"), filename);
> -	strbuf_release(&buf);
> +	if (ref_transaction_update(transaction, ref, commit->object.sha1,
> +				   NULL, REF_NODEREF, NULL,
> +				   &err))
> +		die(_("Could not write ref %s: %s"), ref, err.buf);
> +
> +	if (ref_transaction_commit(transaction, &err))
> +		die(_("Could not commit ref write %s: %s"), ref, err.buf);
>  }

I didn't check all the details, but this code looks a lot like what
update_ref() does. Maybe you can use that function?

>  static void print_advice(int show_hint, struct replay_opts *opts)
> @@ -186,7 +187,7 @@ static void print_advice(int show_hint, struct replay_opts *opts)
> [...]

Otherwise, looks good to me.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu