[PATCH/RFC] [git-subtree.sh] Use raw subject and body modifier "%B" instead of "%s%n%n%b" for commit

[PATCH/RFC] [git-subtree.sh] Use raw subject and body modifier "%B" instead of "%s%n%n%b" for commit

[Techlive Zheng]

I don't know if it is the right place to post this patch, I have sended
an email to the original author apenwarr and have no response. According
to <https://github.com/apenwarr/git-subtree/blob/master/THIS-REPO-IS-OBSOLETE>,
this is the place, but <https://github.com/git/git/blob/master/contrib/README> says
different, which is really confusing. Anyway, here I am.

Recently, I imported a foreign git project as a sub directory into a
main repo which I intend to maintain as primary.

Due to the project I imported has its own remote repo which hosted
on the github, I expected after a 'git-subtree.sh split' the newly
generated subtree branch would be exactly identical to the original
branch. Unfortunately, it is not. I have fixed the committer date
and make everything looks the same with the original branch, but
they just did not end up with same commit sha1 hash. Then, I used
`git cat-file -p` to view the raw output of the both commits and
found that the commit generate by git-subtree has a extra 'new-line'
character appended at the end of the subject which causes the problem.

I checked the source and found "%s%n%n%b" were used to generate the
commit message, this works the fine when a commit has a subject as
well as a body, but most of my commits only have a subject under
which condition a extra 'new-line' character is appended.

Instead, a raw subject and body message modifier '%B' should be used.

Though I think this patch should be applied by default, but the mistake
has been there for a long time, applying this patch may cause the patched
git-subtree generate a different branch for those whose subtree branch
has already been generated using the old git-subtree. Maybe this should
be explained in the help or man page, and add a condition check or a
compatible mode somehow.

Techlive Zheng (1):
  subtree.sh: Use raw subject and body modifier "%B" instead of "%s%n%n%b"

 contrib/subtree/git-subtree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.7.11.4

[PATCH] subtree.sh: Use raw subject and body modifier "%B" instead of "%s%n%n%b"

[Techlive Zheng]

"%s%n%n%b" is not always equal to "%B". If the commit msg does not have
a body, this will append an extra new-line character to the msg title
which would cause the splited commit has a new sha1 hash. In most cases,
this does not matter, but for a project which did not merged using this
script initially, the 'split' command would not genereate the same
commits as the orginal which may cause conflicts.

Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
---
 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 920c664..5598210 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -296,7 +296,7 @@ copy_commit()
 	# We're going to set some environment vars here, so
 	# do it in a subshell to get rid of them safely later
 	debug copy_commit "{$1}" "{$2}" "{$3}"
-	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' "$1" |
+	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" |
 	(
 		read GIT_AUTHOR_NAME
 		read GIT_AUTHOR_EMAIL
-- 
1.7.11.4

Re: [PATCH/RFC] [git-subtree.sh] Use raw subject and body modifier "%B" instead of "%s%n%n%b" for commit

[greened]

Techlive Zheng <techlivezheng@gmail.com> writes:

> I don't know if it is the right place to post this patch, I have sended
> an email to the original author apenwarr and have no response. According
> to <https://github.com/apenwarr/git-subtree/blob/master/THIS-REPO-IS-OBSOLETE>,
> this is the place, but <https://github.com/git/git/blob/master/contrib/README> says
> different, which is really confusing. Anyway, here I am.

This is the place.

> Recently, I imported a foreign git project as a sub directory into a
> main repo which I intend to maintain as primary.

Ok.

> Due to the project I imported has its own remote repo which hosted
> on the github, I expected after a 'git-subtree.sh split' the newly
> generated subtree branch would be exactly identical to the original
> branch. 

I would have thought so too.

> Unfortunately, it is not. I have fixed the committer date and make
> everything looks the same with the original branch, but they just did
> not end up with same commit sha1 hash. Then, I used `git cat-file -p`
> to view the raw output of the both commits and found that the commit
> generate by git-subtree has a extra 'new-line' character appended at
> the end of the subject which causes the problem.

Hmm.

> I checked the source and found "%s%n%n%b" were used to generate the
> commit message, this works the fine when a commit has a subject as
> well as a body, but most of my commits only have a subject under
> which condition a extra 'new-line' character is appended.

Ah.  Yes, we should fix this.

> Instead, a raw subject and body message modifier '%B' should be used.

Ok.

> Though I think this patch should be applied by default, but the mistake
> has been there for a long time, applying this patch may cause the patched
> git-subtree generate a different branch for those whose subtree branch
> has already been generated using the old git-subtree. Maybe this should
> be explained in the help or man page, and add a condition check or a
> compatible mode somehow.

The problem is in the split code?  I'm not sure this is a big issue.  I
can run some experiments.

                      -Dave

Re: [PATCH] subtree.sh: Use raw subject and body modifier "%B" instead of "%s%n%n%b"

[greened]

Techlive Zheng <techlivezheng@gmail.com> writes:

> "%s%n%n%b" is not always equal to "%B". If the commit msg does not have
> a body, this will append an extra new-line character to the msg title
> which would cause the splited commit has a new sha1 hash. In most cases,
> this does not matter, but for a project which did not merged using this
> script initially, the 'split' command would not genereate the same
> commits as the orginal which may cause conflicts.
>
> Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
> ---
>  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 920c664..5598210 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -296,7 +296,7 @@ copy_commit()
>  	# We're going to set some environment vars here, so
>  	# do it in a subshell to get rid of them safely later
>  	debug copy_commit "{$1}" "{$2}" "{$3}"
> -	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' "$1" |
> +	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" |
>  	(
>  		read GIT_AUTHOR_NAME
>  		read GIT_AUTHOR_EMAIL

This looks good to me.  I assume this passes all the tests.  Can you add
a test for this bug so we don't regress?  Junio, I am good with this
patch as soon as we get a test for the problem.

Thanks!

                             -Dave

Re: [PATCH] subtree.sh: Use raw subject and body modifier "%B" instead of "%s%n%n%b"

[greened]

greened@obbligato.org writes:

> Techlive Zheng <techlivezheng@gmail.com> writes:
>
>> "%s%n%n%b" is not always equal to "%B". If the commit msg does not have
>> a body, this will append an extra new-line character to the msg title
>> which would cause the splited commit has a new sha1 hash. In most cases,
>> this does not matter, but for a project which did not merged using this
>> script initially, the 'split' command would not genereate the same
>> commits as the orginal which may cause conflicts.
>>
>> Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
>> ---
>>  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 920c664..5598210 100755
>> --- a/contrib/subtree/git-subtree.sh
>> +++ b/contrib/subtree/git-subtree.sh
>> @@ -296,7 +296,7 @@ copy_commit()
>>  	# We're going to set some environment vars here, so
>>  	# do it in a subshell to get rid of them safely later
>>  	debug copy_commit "{$1}" "{$2}" "{$3}"
>> -	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' "$1" |
>> +	git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" |
>>  	(
>>  		read GIT_AUTHOR_NAME
>>  		read GIT_AUTHOR_EMAIL
>
> This looks good to me.  I assume this passes all the tests.  Can you add
> a test for this bug so we don't regress?  Junio, I am good with this
> patch as soon as we get a test for the problem.

I've applied this patch to my working copy but I'm not finding that I
can recreate the original problem when the patch is disabled.

I assumed the scenario you're trying to fix is:

- Make some commit C to project A with a one-line message
- work, commit, work...
- Add project A as a subproject
- work, commit, work...
- Split project A off into a separate repository

After this, commit C with the one-line message in the split-off projet
should have the same hash it had before project A was incorporated as a
subproject.

As I understad it, you saw the post-split commit having a different
hash?

Is that right?  I am not seeing that problem even without your patch.

I want to make sure I understand what the problem is so I can test for
it.

Thanks!

                      -David

Re: [PATCH] subtree.sh: Use raw subject and body modifier "%B" instead of "%s%n%n%b"

[greened]

greened@obbligato.org writes:

> I've applied this patch to my working copy but I'm not finding that I
> can recreate the original problem when the patch is disabled.
     ^ not

                          -David