* [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin
@ 2016-07-21 0:56 Brett Cundal
2016-07-21 5:26 ` David Aguilar
0 siblings, 1 reply; 6+ messages in thread
From: Brett Cundal @ 2016-07-21 0:56 UTC (permalink / raw)
To: git
---
contrib/subtree/git-subtree.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7a39b30..556cd92 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -661,7 +661,7 @@ cmd_split()
if [ -n "$rejoin" ]; then
debug "Merging split branch into HEAD..."
latest_old=$(cache_get latest_old)
- git merge -s ours \
+ git merge -s ours --allow-unrelated-histories \
-m "$(rejoin_msg "$dir" $latest_old $latest_new)" \
$latest_new >&2 || exit $?
fi
--
https://github.com/git/git/pull/274
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin
2016-07-21 0:56 [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin Brett Cundal
@ 2016-07-21 5:26 ` David Aguilar
2016-07-21 15:25 ` Johannes Schindelin
0 siblings, 1 reply; 6+ messages in thread
From: David Aguilar @ 2016-07-21 5:26 UTC (permalink / raw)
To: Brett Cundal; +Cc: git, Roberto Tyley
[cc'd Roberto for submitGit q's]
On Thu, Jul 21, 2016 at 12:56:51AM +0000, Brett Cundal wrote:
> ---
<emtpy commit message>
The message on the pull request[1] has a better justification
for this change, which would have been nice in the commit
message itself:
Git 2.9 added a check against merging unrelated histories, which
is exactly what git subtree with --rejoin does. Adding the
--allow-unrelated-histories flag to merge will override this
check.
Is it possible that maybe submitGit can detect an empty commit
message for single-commit PRs and transplant that message onto it?
As-is, the commit itself should probably be amended to contain
that information.
> contrib/subtree/git-subtree.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 7a39b30..556cd92 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -661,7 +661,7 @@ cmd_split()
> if [ -n "$rejoin" ]; then
> debug "Merging split branch into HEAD..."
> latest_old=$(cache_get latest_old)
> - git merge -s ours \
> + git merge -s ours --allow-unrelated-histories \
> -m "$(rejoin_msg "$dir" $latest_old $latest_new)" \
> $latest_new >&2 || exit $?
> fi
>
> --
With the above description this change makes more sense,
but it seems that the existing tests do not detect the breakage
fixed by this patch.
Can you please add a test case in t/t7900-subtree.sh
demonstrating the breakage?
Looks good otherwise.
[1] https://github.com/git/git/pull/274
--
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin
2016-07-21 5:26 ` David Aguilar
@ 2016-07-21 15:25 ` Johannes Schindelin
2016-07-21 18:09 ` Brett Cundal
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2016-07-21 15:25 UTC (permalink / raw)
To: David Aguilar; +Cc: Brett Cundal, git, Roberto Tyley
Hi David,
On Wed, 20 Jul 2016, David Aguilar wrote:
> As-is, the commit itself should probably be amended to contain
> that information [the better explanation].
Definitely. Keep in mind: if this gets merged or cherry-picked elsewhere,
the Pull Request's message is just as lost.
Ciao,
Johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin
2016-07-21 15:25 ` Johannes Schindelin
@ 2016-07-21 18:09 ` Brett Cundal
2016-07-22 8:11 ` Johannes Schindelin
0 siblings, 1 reply; 6+ messages in thread
From: Brett Cundal @ 2016-07-21 18:09 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: David Aguilar, git, Roberto Tyley
[re-sending as plain-text so the list software doesn't reject as
spam... what decade is this?]
Hey,
Sorry about the empty commit message... there was one originally
(albeit not as detailed as the pull request), but I guess it got
stripped? As you have probably guessed, I have no idea how you guys do
patch submission.
I'm not currently able to spend much time getting up to speed with git
development just to submit this fix... I would have just filed a bug,
but I couldn't figure out how (I guess it's just this mailing list?)
and I figured a patch would be more useful. I had a look at writing a
test initially, but I didn't see any existing tests for 'git subtree',
and I'm not familiar with the test framework.
The case that fails is basically any usage of split --rejoin... e.g.
make a new repo, make changes to directory a, make changes to
directory b... do a 'git subtree split -P a --rejoin' and it will fail
at the end when attempting to merge the split commits back to the main
line, due to the two lines being unrelated (now disallowed by default
in git 2.9).
Hopefully I'm not just doing something wrong - I'm surprised such a
major bug has not been fixed already... I guess noone else uses
--rejoin? Anyhow, this patch fixes the issue for me.
Hope that helps,
-- Brett
On 21 July 2016 at 08:25, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi David,
>
> On Wed, 20 Jul 2016, David Aguilar wrote:
>
>> As-is, the commit itself should probably be amended to contain
>> that information [the better explanation].
>
> Definitely. Keep in mind: if this gets merged or cherry-picked elsewhere,
> the Pull Request's message is just as lost.
>
> Ciao,
> Johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin
2016-07-21 18:09 ` Brett Cundal
@ 2016-07-22 8:11 ` Johannes Schindelin
2016-07-22 17:15 ` Brett Cundal
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2016-07-22 8:11 UTC (permalink / raw)
To: Brett Cundal; +Cc: David Aguilar, git, Roberto Tyley
Hi Brett,
On Thu, 21 Jul 2016, Brett Cundal wrote:
> [re-sending as plain-text so the list software doesn't reject as
> spam... what decade is this?]
I know it feels good to get frustration off of your chest by ranting. I am
very sympathetic to that. Please note, however, that it puts the people
who are ready to help you immediately into a defensive corner. Probably
unintentional?
> Sorry about the empty commit message... there was one originally (albeit
> not as detailed as the pull request), but I guess it got stripped? As
> you have probably guessed, I have no idea how you guys do patch
> submission.
It is okay. For historical reasons, the patch submission is a bit more
involved than simply opening a Pull Request. We also frown upon
top-posting, among other rules of netiquette. You may dislike it, but you
would have to build more of a "street cred" if you truly wanted to try to
convince the Git developers to change it.
Back to the patch you wanted to submit: since this is an important bug
fix, I spent the time to clean it up. The only missing bit that requires
further effort from your side is that we need your Sign-off. See
https://github.com/git/git/blob/v2.9.2/Documentation/SubmittingPatches#L239-L291
for an explanation. Essentially, you are stating explicitly that you are
not legally prohibited from contributing this patch. If you can say that,
simply reply with a
Signed-off-by: Brett Cundal <brett.cundal@iugome.com>
We can take it from there by inserting it into the following patch:
-- snipsnap --
From: Brett Cundal <brett.cundal@iugome.com>
Subject: [PATCH] subtree: allow unrelated histories when splitting with --rejoin
Git 2.9 added a check against merging unrelated histories, which is
exactly what git subtree with --rejoin does. Adding the
--allow-unrelated-histories flag to merge will override this check.
---
contrib/subtree/git-subtree.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7a39b30..556cd92 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -661,7 +661,7 @@ cmd_split()
if [ -n "$rejoin" ]; then
debug "Merging split branch into HEAD..."
latest_old=$(cache_get latest_old)
- git merge -s ours \
+ git merge -s ours --allow-unrelated-histories \
-m "$(rejoin_msg "$dir" $latest_old $latest_new)" \
$latest_new >&2 || exit $?
fi
--
2.9.0.281.g286a8d9
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin
2016-07-22 8:11 ` Johannes Schindelin
@ 2016-07-22 17:15 ` Brett Cundal
0 siblings, 0 replies; 6+ messages in thread
From: Brett Cundal @ 2016-07-22 17:15 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: David Aguilar, git, Roberto Tyley
On 22 July 2016 at 01:11, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> I know it feels good to get frustration off of your chest by ranting. I am
> very sympathetic to that. Please note, however, that it puts the people
> who are ready to help you immediately into a defensive corner. Probably
> unintentional?
Shortest rant ever. :) Sorry if I caused any offense. It just seemed very odd to
block multi-part MIME that happened to contain a text/html part in addition to
text/plain, given that (AFAIK) the vast majority of email clients in
recent years
produce such a thing. Anyhow, off-topic... your list policy is your business.
> Back to the patch you wanted to submit: since this is an important bug
> fix, I spent the time to clean it up. The only missing bit that requires
> further effort from your side is that we need your Sign-off. See
> https://github.com/git/git/blob/v2.9.2/Documentation/SubmittingPatches#L239-L291
> for an explanation. Essentially, you are stating explicitly that you are
> not legally prohibited from contributing this patch. If you can say that,
> simply reply with a
>
> Signed-off-by: Brett Cundal <brett.cundal@iugome.com>
>
> We can take it from there by inserting it into the following patch:
So, again I'll have to apologise... I should have submitted this as a bug
report rather than a patch, since the ownership is not legally clear. Didn't
even occur to me for such a trivial fix. If you can just treat this as
a bug report
at this point and 'reimplement' it, that would probably be the
simplest thing for
everyone... I guess this may be tricky since there are limited ways to possibly
implement this, but for the same reason it would be impossible IMO for me or
anyone else to legally claim authorship of it (but IANAL). If this is
going to cause
this fix to be jammed in limbo for all eternity, then let me know and
I'll see what
I can do to get the proper authorization.
-- Brett
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-22 17:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-21 0:56 [PATCH] git-subtree.sh: Use --allow-unrelated-histories when splitting with --rejoin Brett Cundal
2016-07-21 5:26 ` David Aguilar
2016-07-21 15:25 ` Johannes Schindelin
2016-07-21 18:09 ` Brett Cundal
2016-07-22 8:11 ` Johannes Schindelin
2016-07-22 17:15 ` Brett Cundal
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).