* [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
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 NNTP newsgroup(s).