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

[David Turner]

On Thu, 2015-06-25 at 14:11 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > 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 
> > ---
> 
> I may have said this already in the last round, but I cannot shake
> off the feeling that this patch is doing two completely unrelated
> things at once.
> 
> The change to refs.c that introduced the should_autocreate_reflog()
> helper (which is a good idea even without any other changes in this
> patch) 

I'll break that out into a separate patch when I reroll.

> and then using that to commit_ref_update() does not have
> anything to do with CHERRY_PICK_HEAD/REVERT_HEAD, does it?

I had thought that it did, but some testing shows that it does not (or
at least does not yet).  The original series of patches was assembled
out of a long process of making the test suite pass with the alternate
ref backend.  I expect that as I rewrite that series, I'll either figure
out what issues these change solved, or remove them.  In this case, I'll
split the patch.

>  * commit_ref_update() gained a new "flags" parameter, but it does
>not seem to be used at all.  Why?

cruft, will remove.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

David Turner  writes:

> 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 
> ---

I may have said this already in the last round, but I cannot shake
off the feeling that this patch is doing two completely unrelated
things at once.

The change to refs.c that introduced the should_autocreate_reflog()
helper (which is a good idea even without any other changes in this
patch) and then using that to commit_ref_update() does not have
anything to do with CHERRY_PICK_HEAD/REVERT_HEAD, does it?

 * commit_ref_update() gained a new "flags" parameter, but it does
   not seem to be used at all.  Why?

 * Why is it a good idea to move the "does the log exist or should
   we auto-create?" check from log_ref_setup() to
   commit_ref_update()?

These are not just unexplained, but does not have anything to do
with using ref API to write and read CHERRY_PICK_HEAD/REVERT_HEAD.

They _might_ be a prerequisite, but it is not clear why.  I suspect
that you do not have to touch refs.c at all for the purpose of the
"theme" of the patch explained by the "Subject" line.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/6] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs

[David Turner]

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 
---
 branch.c |  4 ++--
 builtin/commit.c |  6 +++---
 builtin/merge.c  |  2 +-
 contrib/completion/git-prompt.sh |  4 ++--
 git-gui/lib/commit.tcl   |  2 +-
 refs.c   | 32 +++-
 sequencer.c  | 37 +++--
 t/t7509-commit.sh|  4 ++--
 wt-status.c  |  6 ++
 9 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/branch.c b/branch.c
index b002435..ec598aa 100644
--- a/branch.c
+++ b/branch.c
@@ -302,8 +302,8 @@ void create_branch(const char *head,
 
 void remove_branch_state(void)
 {
-   unlink(git_path("CHERRY_PICK_HEAD"));
-   unlink(git_path("REVERT_HEAD"));
+   delete_ref("CHERRY_PICK_HEAD", NULL, REF_NODEREF);
+   delete_ref("REVERT_HEAD", NULL, REF_NODEREF);
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_RR"));
unlink(git_path("MERGE_MSG"));
diff --git a/builtin/commit.c b/builtin/commit.c
index b5b1158..53c7e90 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -168,7 +168,7 @@ static void determine_whence(struct wt_status *s)
 {
if (file_exists(git_path("MERGE_HEAD")))
whence = FROM_MERGE;
-   else if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
+   else if (ref_exists("CHERRY_PICK_HEAD")) {
whence = FROM_CHERRY_PICK;
if (file_exists(git_path(SEQ_DIR)))
sequencer_in_use = 1;
@@ -1777,8 +1777,8 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
}
ref_transaction_free(transaction);
 
-   unlink(git_path("CHERRY_PICK_HEAD"));
-   unlink(git_path("REVERT_HEAD"));
+   delete_ref("CHERRY_PICK_HEAD", NULL, REF_NODEREF);
+   delete_ref("REVERT_HEAD", NULL, REF_NODEREF);
unlink(git_path("MERGE_HEAD"));
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
diff --git a/builtin/merge.c b/builtin/merge.c
index 46aacd6..3e2ae2f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1206,7 +1206,7 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
else
die(_("You have not concluded your merge (MERGE_HEAD 
exists)."));
}
-   if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
+   if (ref_exists("CHERRY_PICK_HEAD")) {
if (advice_resolve_conflict)
die(_("You have not concluded your cherry-pick 
(CHERRY_PICK_HEAD exists).\n"
"Please, commit your changes before you merge."));
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 366f0bc..e2c5583 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -415,9 +415,9 @@ __git_ps1 ()
fi
elif [ -f "$g/MERGE_HEAD" ]; then
r="|MERGING"
-   elif [ -f "$g/CHERRY_PICK_HEAD" ]; then
+   elif git rev-parse --quiet --verify "CHERRY_PICK_HEAD" 
>/dev/null; then
r="|CHERRY-PICKING"
-   elif [ -f "$g/REVERT_HEAD" ]; then
+   elif git rev-parse --quiet --verify "REVERT_HEAD" >/dev/null; 
then
r="|REVERTING"
elif [ -f "$g/BISECT_LOG" ]; then
r="|BISECTING"
diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
index 864b687..2b08b13 100644
--- a/git-gui/lib/commit.tcl
+++ b/git-gui/lib/commit.tcl
@@ -409,7 +409,7 @@ A rescan will be automatically started now.
catch {file delete [gitdir MERGE_MSG]}
catch {file delete [gitdir SQUASH_MSG]}
catch {file delete [gitdir GITGUI_MSG]}
-   catch {file delete [gitdir CHERRY_PICK_HEAD]}
+   catch {git update-ref -d --no-deref CHERRY_PICK_HEAD}
 
# -- Let rerere do its thing.
#
diff --git a/refs.c b/refs.c
index b34a54a..de7b5ef 100644
--- a/refs.c
+++ b/refs.c
@@ -2979,7 +2979,7 @@ static int write_ref_to_lockfile(struct ref_lock *lock,
 const unsigned char *sha1, struct strbuf* err);
 static int commit_ref_update(struct ref_lock *lock,
 const unsigned char *sha1, const char *logmsg,
-struct strbuf *err);
+struct strbuf *err, int flags);
 
 int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
 {
@@ -3041,7 +3041,7 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
hashcpy(lock->old_oid.hash, orig_sha1);
 
if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
-   commit_ref_update(lock, orig_sha1, logmsg, &err)) {
+