Re: [PATCH v3 00/16] Use merge_recursive() directly in the builtin am
Johannes Schindelin writes: > I do not need cc/apply-am at all, it turns out, but my patch series has a > minor conflict with 'jc/renormalize-merge-kill-safer-crlf'. > > Since you indicated that you want to cook that branch a bit in 'next' > first, I will rebase to master+bc/cocci+js/am-call-theirs-theirs and > re-submit. Thanks. I suspect the renomalization would graduate earlier than the topic in question, but leaving dependency to the minimum and have rerere take care of minor conflicts has proved to be a better approach in general over time; the base you chose above sounds appropriate. Thanks. -- 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 v3 00/16] Use merge_recursive() directly in the builtin am
Hi Junio, On Tue, 19 Jul 2016, Johannes Schindelin wrote: > On Mon, 18 Jul 2016, Junio C Hamano wrote: > > > Junio C Hamano writes: > > > > You can assume that I'll merge bc/cocci and > > js/am-call-theirs-theirs-in-fallback-3way to 'master' during that time, > > so an appropriate base to use would be the result of > > > > git checkout master^0 > > git merge bc/cocci > > git merge js/am-call-theirs-theirs-in-fallback-3way > > git merge cc/apply-am > > > > if you want a semi-solid base to rebuild am-3-merge-recursive-direct > > on. I do not need cc/apply-am at all, it turns out, but my patch series has a minor conflict with 'jc/renormalize-merge-kill-safer-crlf'. Since you indicated that you want to cook that branch a bit in 'next' first, I will rebase to master+bc/cocci+js/am-call-theirs-theirs and re-submit. Ciao, Dscho -- 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 v3 00/16] Use merge_recursive() directly in the builtin am
Hi Junio, On Mon, 18 Jul 2016, Junio C Hamano wrote: > Junio C Hamano writes: > > > Johannes Schindelin writes: > > > >> The two topics that are in 'pu' and conflict with this series are > >> 'jh/clean-smudge-annex' and 'bc/cocci'. > >> > >> It also conflicted with 'va/i18n-even-more', but that one was merged to > >> 'master'. > >> > >> Now, I think it would be okay to wait for 'bc/cocci' to go to 'master' > >> before integrating the 'am-3-merge-recursive-direct' branch, but I would > >> want to avoid waiting for 'jh/clean-smudge-annex'. > > Nothing seems to be happening on jh/clean-smudge-annex front, so > unless we hear otherwise from Joey in the coming couple of days, > let's get js/am-3-merge-recursive-direct untangled from its > dependencies and get it into a shape to hit 'next'. Okay, I will rebase and re-submit. > You can assume that I'll merge bc/cocci and > js/am-call-theirs-theirs-in-fallback-3way to 'master' during that time, > so an appropriate base to use would be the result of > > git checkout master^0 > git merge bc/cocci > git merge js/am-call-theirs-theirs-in-fallback-3way > git merge cc/apply-am > > if you want a semi-solid base to rebuild am-3-merge-recursive-direct > on. Okay. The way my `mail-patch-series.sh` script works, however, is that it infers the base commit by testing which tip among pu, next and master is the most appropriate. So I guess I will have to hack it up a bit to allow basing my patch series on something custom. > I am not 100% sure about the doneness of cc/apply-am yet, though but it > could be that am-3-merge-recursive-direct does not have to depend on it > at all. You'd know better than I do ;-) I agree on the doneness of cc/apply-am (as you know, I tried to help it along but my comments had an adverse effect). And no: nothing in my entire rebase--helper patches relies on cc/apply-am (I do not even believe that anything conflicts with it, either). > After that, I'll see if the conflict resolution is manageable when > remerging jh/clean-smudge-annex to 'pu', and if it is not, I'll worry > about it at that point. I can help with that, too. Ciao, Dscho -- 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 v3 00/16] Use merge_recursive() directly in the builtin am
Junio C Hamano writes: > Johannes Schindelin writes: > >> The two topics that are in 'pu' and conflict with this series are >> 'jh/clean-smudge-annex' and 'bc/cocci'. >> >> It also conflicted with 'va/i18n-even-more', but that one was merged to >> 'master'. >> >> Now, I think it would be okay to wait for 'bc/cocci' to go to 'master' >> before integrating the 'am-3-merge-recursive-direct' branch, but I would >> want to avoid waiting for 'jh/clean-smudge-annex'. Nothing seems to be happening on jh/clean-smudge-annex front, so unless we hear otherwise from Joey in the coming couple of days, let's get js/am-3-merge-recursive-direct untangled from its dependencies and get it into a shape to hit 'next'. You can assume that I'll merge bc/cocci and js/am-call-theirs-theirs-in-fallback-3way to 'master' during that time, so an appropriate base to use would be the result of git checkout master^0 git merge bc/cocci git merge js/am-call-theirs-theirs-in-fallback-3way git merge cc/apply-am if you want a semi-solid base to rebuild am-3-merge-recursive-direct on. I am not 100% sure about the doneness of cc/apply-am yet, though but it could be that am-3-merge-recursive-direct does not have to depend on it at all. You'd know better than I do ;-) After that, I'll see if the conflict resolution is manageable when remerging jh/clean-smudge-annex to 'pu', and if it is not, I'll worry about it at that point. Thanks. -- 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 v3 00/16] Use merge_recursive() directly in the builtin am
Johannes Schindelin writes: > The two topics that are in 'pu' and conflict with this series are > 'jh/clean-smudge-annex' and 'bc/cocci'. > > It also conflicted with 'va/i18n-even-more', but that one was merged to > 'master'. > > Now, I think it would be okay to wait for 'bc/cocci' to go to 'master' > before integrating the 'am-3-merge-recursive-direct' branch, but I would > want to avoid waiting for 'jh/clean-smudge-annex'. > > Do you concur? If so, I will rebase onto 'master' as soon as 'bc/cocci' > lands in there. I do not have a strong preference myself. As you are not proposing to eject jh/clean-smudge-annex from my tree, I'd have to resolve the conflicts when the second topic is merged after one topic, whichever happens to be more ready. IOW, such a rebase adds to my workload. Like it or not, it is normal for different topics to want to touch the overlapping area or one topic to invalidate an assumption the other topic is based on, and working well together does not have to mean leaving the conflict resolution to the sole responsibility of the integrator. A clean way forward may be for everybody involved (and I see you forgot to add Joey to this thread) to agree that this topic is more ready than jh/clean-smudge-annex and you and Joey to work together to rebase jh/clean-smudge-annex on top of this topic (or possibly the other way around), making him wait for you. -- 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 v3 00/16] Use merge_recursive() directly in the builtin am
Hi Junio, On Tue, 12 Jul 2016, Junio C Hamano wrote: > Johannes Schindelin writes: > > > This is the second iteration of the long-awaited re-roll of the attempt to > > avoid spawning merge-recursive from the builtin am and use merge_recursive() > > directly instead. > > This is actually the third iteration. It is. > I am trying to tease dependencies apart and apply this on a more > reasonable base than a commit that happened to be at 'pu' on one > day, but this would probably take some time, and I may give up > merging it anywhere for today's integration cycle. We'll see. The two topics that are in 'pu' and conflict with this series are 'jh/clean-smudge-annex' and 'bc/cocci'. It also conflicted with 'va/i18n-even-more', but that one was merged to 'master'. Now, I think it would be okay to wait for 'bc/cocci' to go to 'master' before integrating the 'am-3-merge-recursive-direct' branch, but I would want to avoid waiting for 'jh/clean-smudge-annex'. Do you concur? If so, I will rebase onto 'master' as soon as 'bc/cocci' lands in there. Ciao, Dscho -- 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 v3 00/16] Use merge_recursive() directly in the builtin am
Johannes Schindelin writes: > This is the second iteration of the long-awaited re-roll of the attempt to > avoid spawning merge-recursive from the builtin am and use merge_recursive() > directly instead. This is actually the third iteration. I am trying to tease dependencies apart and apply this on a more reasonable base than a commit that happened to be at 'pu' on one day, but this would probably take some time, and I may give up merging it anywhere for today's integration cycle. We'll see. -- 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 v3 00/16] Use merge_recursive() directly in the builtin am
This is the second iteration of the long-awaited re-roll of the attempt to avoid spawning merge-recursive from the builtin am and use merge_recursive() directly instead. The *real* reason for the reroll is that I need a libified recursive merge to accelerate the interactive rebase by teaching the sequencer to do rebase -i's grunt work. In this endeavor, we need to be extra careful to retain backwards compatibility. The test script t6022-merge-rename.sh, for example, verifies that `git pull` exits with status 128 in case of a fatal error. To that end, we need to make sure that fatal errors are handled by existing (builtin) users via exit(128) (or die(), which calls exit(128) at the end). New users (such as a builtin helper doing rebase -i's grunt work) may want to print some helpful advice what happened and how to get out of this mess before erroring out. The changes relative to the second iteration of this patch series: - the was_tracked() function was adjusted as per Junio's suggestions - the "counter gender bias" patch was submitted separately, in $gmane/299009 (please note that the "am -3: use merge_recursive() directly again" patch is now slightly awkward as a consequence) - this patch series is on top of 'pu', to address the conflicts with the 'jh/clean-smudge-annex' and the 'bc/cocci' branches - please note that the interdiff does not show the full picture: I generated it relative to v2 rebased on top of pu (resolving many merge conflicts in the process that are hidden from the interdiff) This patch series touches rather important code. Now that I addressed concerns such as fixing translated bug reports, I would appreciate thorough reviews with a focus on the critical parts of the code, those that could result in regressions. Johannes Schindelin (16): Verify that `git pull --rebase` shows the helpful advice when failing Report bugs consistently Avoid translating bug messages merge-recursive: clarify code in was_tracked() Prepare the builtins for a libified merge_recursive() merge_recursive: abort properly upon errors merge-recursive: avoid returning a wholesale struct merge-recursive: allow write_tree_from_memory() to error out merge-recursive: handle return values indicating errors merge-recursive: switch to returning errors instead of dying am -3: use merge_recursive() directly again merge-recursive: flush output buffer before printing error messages merge-recursive: write the commit title in one go merge-recursive: offer an option to retain the output in 'obuf' Ensure that the output buffer is released after calling merge_trees() merge-recursive: flush output buffer even when erroring out builtin/am.c | 63 +++--- builtin/checkout.c | 5 +- builtin/ls-files.c | 3 +- builtin/merge.c| 2 + builtin/update-index.c | 2 +- grep.c | 8 +- imap-send.c| 4 +- merge-recursive.c | 508 + merge-recursive.h | 2 +- sequencer.c| 5 + sha1_file.c| 4 +- t/t5520-pull.sh| 30 +++ trailer.c | 2 +- transport.c| 2 +- wt-status.c| 4 +- 15 files changed, 382 insertions(+), 262 deletions(-) Published-As: https://github.com/dscho/git/releases/tag/am-3-merge-recursive-direct-v3 Interdiff vs v2: diff --git a/builtin/am.c b/builtin/am.c index d626532..8dc4239 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -29,6 +29,7 @@ #include "prompt.h" #include "mailinfo.h" #include "apply.h" +#include "object.h" /** * Returns the length of the first line of msg. @@ -1627,15 +1628,14 @@ static int build_fake_ancestor(const struct am_state *state, const char *index_f */ static int fall_back_threeway(const struct am_state *state, const char *index_path) { - unsigned char orig_tree[GIT_SHA1_RAWSZ], her_tree[GIT_SHA1_RAWSZ], -our_tree[GIT_SHA1_RAWSZ]; - const unsigned char *bases[1] = {orig_tree}; + struct object_id orig_tree, her_tree, our_tree; + const struct object_id *bases[1] = { &orig_tree }; struct merge_options o; struct commit *result; char *her_tree_name; - if (get_sha1("HEAD", our_tree) < 0) - hashcpy(our_tree, EMPTY_TREE_SHA1_BIN); + if (get_oid("HEAD", &our_tree) < 0) + hashcpy(our_tree.hash, EMPTY_TREE_SHA1_BIN); if (build_fake_ancestor(state, index_path)) return error("could not build fake ancestor"); @@ -1643,7 +1643,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa discard_cache(); read_cache_from(index_path); - if (write_index_as_tree(orig_tree, &the_index, index_path, 0, NULL)) + if (write_index_as_tree(orig_tree.hash, &the_index, index_path, 0, NULL)) return error(_("Repository lacks necessary blobs to fa