Re: [PATCH v2 2/2] merge-base: "--reflog" mode finds fork point from reflog entries
Johannes Sixt writes: >> +for count in 1 2 3 4 5 >> +do >> +git merge-base --reflog base $(cat derived$count) >actual && >> +test_cmp expect$count actual || break >> +done && > > This does not work as intended because the exit code of 'break' is always > zero. Unlike 'exit' and 'return', it does *not* pick up the exit code of > the last command that was executed. You are right. I obviously was not thinking straight. > You could put the loops into a function from which you 'return', > but that is obscure in this case. The first iteration was better, > IMO. I do not think using "return 1" is a good thing to do, either. We saw breakages with different shells around the use of "return" and we know the original "exit 1 inside subshell" works reliably everywher. I'll send out a revamped version later today, updating not just the test but the implementation. Thank for a dose of sanity. -- 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/2] merge-base: "--reflog" mode finds fork point from reflog entries
Am 10/25/2013 10:09, schrieb John Keeping:
> On Fri, Oct 25, 2013 at 09:12:10AM +0200, Johannes Sixt wrote:
>> You could put the loops into a function from which you 'return', but that
>> is obscure in this case. The first iteration was better, IMO.
>
> Wouldn't it be simpler to just return from the test? That is, replace
> the "break" in the above patch with "return 1".
Good catch! We explicitly have
test_eval_ () {
# This is a separate function because some tests use
# "return" to end a test_expect_success block early.
eval &3 2>&4 "$*"
}
to protect this use of return.
-- Hannes
--
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/2] merge-base: "--reflog" mode finds fork point from reflog entries
On Fri, Oct 25, 2013 at 09:12:10AM +0200, Johannes Sixt wrote: > Am 10/25/2013 0:21, schrieb Junio C Hamano: > > +test_expect_success 'using reflog to find the fork point' ' > > + git reset --hard && > > + git checkout -b base $E && > > + > > + for count in 1 2 3 4 5 > > + do > > + git commit --allow-empty -m "Base commit #$count" && > > + git rev-parse HEAD >expect$count && > > + git checkout -B derived && > > + git commit --allow-empty -m "Derived #$count" && > > + git rev-parse HEAD >derived$count && > > + git checkout base || break > > + done && > > + > > + for count in 1 2 3 4 5 > > + do > > + git merge-base --reflog base $(cat derived$count) >actual && > > + test_cmp expect$count actual || break > > + done && > > This does not work as intended because the exit code of 'break' is always > zero. Unlike 'exit' and 'return', it does *not* pick up the exit code of > the last command that was executed. > > That's annoying, but makes some sense because 'break 2' does not mean to > apply exit code 2 to the command, either, but to break out of 2 levels of > loops. > > You could put the loops into a function from which you 'return', but that > is obscure in this case. The first iteration was better, IMO. Wouldn't it be simpler to just return from the test? That is, replace the "break" in the above patch with "return 1". There's code in several other test cases (e.g. t3311) which handles the same problem like that. -- 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/2] merge-base: "--reflog" mode finds fork point from reflog entries
Am 10/25/2013 0:21, schrieb Junio C Hamano: > +test_expect_success 'using reflog to find the fork point' ' > + git reset --hard && > + git checkout -b base $E && > + > + for count in 1 2 3 4 5 > + do > + git commit --allow-empty -m "Base commit #$count" && > + git rev-parse HEAD >expect$count && > + git checkout -B derived && > + git commit --allow-empty -m "Derived #$count" && > + git rev-parse HEAD >derived$count && > + git checkout base || break > + done && > + > + for count in 1 2 3 4 5 > + do > + git merge-base --reflog base $(cat derived$count) >actual && > + test_cmp expect$count actual || break > + done && This does not work as intended because the exit code of 'break' is always zero. Unlike 'exit' and 'return', it does *not* pick up the exit code of the last command that was executed. That's annoying, but makes some sense because 'break 2' does not mean to apply exit code 2 to the command, either, but to break out of 2 levels of loops. You could put the loops into a function from which you 'return', but that is obscure in this case. The first iteration was better, IMO. > + > + # check defaulting to HEAD > + git merge-base --reflog base >actual && > + test_cmp expect5 actual > +' > + > test_done -- Hannes -- 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/2] merge-base: "--reflog" mode finds fork point from reflog entries
The "git pull --rebase" command computes the fork point of the
branch being rebased using the reflog entries of the "base" branch
(typically a remote-tracking branch) the branch's work was based on,
in order to cope with the case in which the "base" branch has been
rewound and rebuilt. For example, if the history looked like this:
o---B1
/
---o---o---B2--o---o---o---Base
\
B3
\
Derived
where the current tip of the "base" branch is at Base, but earlier
fetch observed that its tip used to be B3 and then B2 and then B1
before getting to the current commit, and the branch being rebased
on top of the latest "base" is based on commit B3, it tries to find
B3 by going through the output of "git rev-list --reflog base" (i.e.
Base, B1, B2, B3) until it finds a commit that is an ancestor of the
current tip "Derived".
Internally, we have get_merge_bases_many() that can compute this
with one-go. We would want a merge-base between Derived and a
fictitious merge commit that would result by merging all the
historical tips of "base". When such a commit exist, we should get
a single result, which exactly match one of the reflog entries of
"base".
Teach "git merge-base" a new mode, "--reflog", to compute exactly
that.
Helped-by: Eric Sunshine
Signed-off-by: Junio C Hamano
---
* With updated tests, based on conversation with Eric Sunshine
builtin/merge-base.c | 77 +++
t/t6010-merge-base.sh | 25 +
2 files changed, 102 insertions(+)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index d39c910..7b9bc15 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -1,6 +1,7 @@
#include "builtin.h"
#include "cache.h"
#include "commit.h"
+#include "refs.h"
#include "parse-options.h"
static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
@@ -27,6 +28,7 @@ static const char * const merge_base_usage[] = {
N_("git merge-base [-a|--all] --octopus ..."),
N_("git merge-base --independent ..."),
N_("git merge-base --is-ancestor "),
+ N_("git merge-base --reflog []"),
NULL
};
@@ -85,6 +87,73 @@ static int handle_is_ancestor(int argc, const char **argv)
return 1;
}
+struct rev_collect {
+ struct commit **commit;
+ int nr;
+ int alloc;
+};
+
+static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+ const char *ident, unsigned long timestamp,
+ int tz, const char *message, void *cbdata_)
+{
+ struct rev_collect *revs = cbdata_;
+ struct commit *commit = lookup_commit(nsha1);
+ if (commit) {
+ ALLOC_GROW(revs->commit, revs->nr + 1, revs->alloc);
+ revs->commit[revs->nr++] = commit;
+ }
+ return 0;
+}
+
+static int handle_reflog(int argc, const char **argv)
+{
+ unsigned char sha1[20];
+ char *refname;
+ const char *commitname;
+ struct rev_collect revs;
+ struct commit *derived;
+ struct commit_list *bases;
+ int i;
+
+ switch (dwim_ref(argv[0], strlen(argv[0]), sha1, &refname)) {
+ case 0:
+ die("No such ref: '%s'", argv[0]);
+ case 1:
+ break; /* good */
+ default:
+ die("Ambiguous refname: '%s'", argv[0]);
+ }
+
+ commitname = (argc == 2) ? argv[1] : "HEAD";
+ if (get_sha1(commitname, sha1))
+ die("Not a valid object name: '%s'", commitname);
+
+ derived = lookup_commit_reference(sha1);
+ memset(&revs, 0, sizeof(revs));
+ for_each_reflog_ent(refname, collect_one_reflog_ent, &revs);
+
+ bases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);
+
+ /*
+* There should be one and only one merge base, when we found
+* a common ancestor among reflog entries.
+*/
+ if (!bases || bases->next)
+ return 1;
+
+ /* And the found one must be one of the reflog entries */
+ for (i = 0; i < revs.nr; i++)
+ if (&bases->item->object == &revs.commit[i]->object)
+ break; /* found */
+ if (revs.nr <= i)
+ return 1; /* not found */
+
+ printf("%s\n", sha1_to_hex(bases->item->object.sha1));
+ free_commit_list(bases);
+ return 0;
+}
+
int cmd_merge_base(int argc, const char **argv, const char *prefix)
{
struct commit **rev;
@@ -100,6 +169,8 @@ int cmd_merge_base(int argc, const char **argv, const char
*prefix)
N_("list revs not reachable from others"), 'r'),
OPT_CMDMODE(0, "is-ancestor", &cmdmode,
N_("is the first one ancestor of the other?"), 'a'),
+ OPT_CMDMODE(0, "reflog", &cmdmode,
+ N_("find where forke
