[RFC PATCH 0/1] *** Add branchname in commit header ***

[RFC PATCH 0/1] *** Add branchname in commit header ***

[Arnaud Bertrand]

From: Arnaud Bertrand <xda@abalgo.com>

For tracability purpose it is often necessary to know which
commit is envolved in a branch
Keeping track of the branchname in the commit header
will make this traceability easy and will facilitate
the graphical toolis that represent the branches and
that have today to use complex algorithm to try to
determine the branch of a commit that was known at
the commit time.

no big change in the code, today rebase is not considered yet
I'm waiting feedback about that before touching
the rebase code.


Arnaud Bertrand (1):
  Add branchname in commit header

 Documentation/pretty-formats.txt |  1 +
 commit.c                         | 11 +++++++++++
 pretty.c                         | 15 +++++++++++++++
 3 files changed, 27 insertions(+)

-- 
2.25.0.rc0.7.g17b02bf28a

[RFC PATCH 1/1] Add branchname in commit header

[Arnaud Bertrand]

From: Arnaud Bertrand <xda@abalgo.com>

Add the branchname in the commit header before the commit message
the following line is added:

branch <branchname>

where <branchname> comes from the function resolve_ref_unsafe("HEAD",...)
without the prefix refs/heads/
A placeholder is added to the pretty format "%Xb" to print the branch information,
X if for "extra-header" and can be use in the future for new features
b is of course for "branch"

the %Xb returns an empty string when branchname information is not found
---
 Documentation/pretty-formats.txt |  1 +
 commit.c                         | 11 +++++++++++
 pretty.c                         | 15 +++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 1a7212ce5a..bd52908f53 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -241,6 +241,7 @@ endif::git-rev-list[]
 '%gE':: reflog identity email (respecting .mailmap, see
 	linkgit:git-shortlog[1] or linkgit:git-blame[1])
 '%gs':: reflog subject
+'%Xb':: branchname in which commit was done
 '%(trailers[:options])':: display the trailers of the body as
 			  interpreted by
 			  linkgit:git-interpret-trailers[1]. The
diff --git a/commit.c b/commit.c
index 434ec030d6..f64a0698be 100644
--- a/commit.c
+++ b/commit.c
@@ -1425,6 +1425,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 	int result;
 	int encoding_is_utf8;
 	struct strbuf buffer;
+	const char *branch = "Unknown";
+	int flags;
+	const char *lbranch =resolve_ref_unsafe("HEAD",0,NULL,&flags);
 
 	assert_oid_type(tree, OBJ_TREE);
 
@@ -1453,6 +1456,14 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		author = git_author_info(IDENT_STRICT);
 	strbuf_addf(&buffer, "author %s\n", author);
 	strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT));
+	if (lbranch) {
+	   skip_prefix(lbranch,"refs/heads/",&branch);
+	   strbuf_addf(&buffer, "branch %s\n", branch);
+	}
+	else {
+	   strbuf_addf(&buffer, "branch Unknown\n");
+	}
+
 	if (!encoding_is_utf8)
 		strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
 
diff --git a/pretty.c b/pretty.c
index 305e903192..5961c39398 100644
--- a/pretty.c
+++ b/pretty.c
@@ -804,6 +804,7 @@ struct format_commit_context {
 
 	/* The following ones are relative to the result struct strbuf. */
 	size_t wrap_start;
+	char *branch;
 };
 
 static void parse_commit_header(struct format_commit_context *context)
@@ -1367,6 +1368,20 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		return 1;
 	}
 
+
+	/* Now add extra header info */
+	if (placeholder[0] == 'X') {
+		switch (placeholder[1]) {
+		case 'b': /* branch ... */
+			c->branch = get_header(msg,"branch");
+			if (c->branch)
+				strbuf_addstr(sb, c->branch);
+			free(c->branch);
+			return 2;
+		}
+	}
+
+
 	/* Now we need to parse the commit message. */
 	if (!c->commit_message_parsed)
 		parse_commit_message(c);
-- 
2.25.0.rc0.7.g17b02bf28a

Re: [RFC PATCH 0/1] *** Add branchname in commit header ***

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 3146 bytes --]

On 2019-12-29 at 22:26:32, Arnaud Bertrand wrote:
> From: Arnaud Bertrand <xda@abalgo.com>
> 
> For tracability purpose it is often necessary to know which
> commit is envolved in a branch
> Keeping track of the branchname in the commit header
> will make this traceability easy and will facilitate
> the graphical toolis that represent the branches and
> that have today to use complex algorithm to try to
> determine the branch of a commit that was known at
> the commit time.
> 
> no big change in the code, today rebase is not considered yet
> I'm waiting feedback about that before touching
> the rebase code.

I encourage you to read back in the history of the list as to why we
haven't done this and why it's not likely to be accepted now, but let me
provide a few reasons of my own.

First, as any contributor to the mailing list can tell you, I am
absolutely terrible at naming things.  I frequently name my branches
something that makes sense to me at the time without regard to whether
that will make sense in the future.  I don't want to memorialize my
momentary thoughtlessness in the history of the repository forever.

Second, one workflow I commonly use is creating a branch with many
commits and then breaking them down into small series that are logical
and easy for review.  If I have a branch called "test-fixes-part7" with
50 commits and then I decide to split that into two branches,
test-fixes-part7 and test-fixes-part8, by copying the branch and using
git reset --hard to truncate the old one, I don't want the old branch
name in my new branch.  A lot of Git workflows assume you can reset and
rename branches this way and having the branch name in the commit header
breaks those workflows.

Third, people reuse branch names.  Right now, I have eight branches with
test fixes all starting with "test-fixes-part" because I'm working on
one major project with all of those test fixes.  However, if a developer
working on another major project also has a lot of changes to the test
suite, they may have lots of identically named branches, which would be
confusing, since our identically named test fix branches would relate to
different projects.  (See my first point.)

However, despite the fact that we aren't likely to add this in the
commit header, there are definitely ways to achieve this.

If you want to include the branch name in the commit, you can do so with
a trailer.  git interpret-trailers can then be used to manipulate and
extract these, and along with a hook, add them automatically if they're
missing.

If you're working on a more centralized project and you want to require
the branch name in your commit trailers, you can set your CI system to
fail or reject commits that don't contain them.  This is the approach
that systems like Gerrit use when the required trailers are missing and
it seems to work reasonably well.

Hopefully these suggestions are helpful in getting you the traceability
you desire without requiring fundamental changes to the way Git works.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [RFC PATCH 0/1] *** Add branchname in commit header ***

[Arnaud Bertrand]

Hello Brian,

Le lun. 30 déc. 2019 à 03:33, brian m. carlson
<sandals@crustytoothpaste.net> a écrit :
>
> On 2019-12-29 at 22:26:32, Arnaud Bertrand wrote:
> > From: Arnaud Bertrand <xda@abalgo.com>
> >
> > For tracability purpose it is often necessary to know which
> > commit is envolved in a branch
> > Keeping track of the branchname in the commit header
> > will make this traceability easy and will facilitate
> > the graphical toolis that represent the branches and
> > that have today to use complex algorithm to try to
> > determine the branch of a commit that was known at
> > the commit time.
> >
> > no big change in the code, today rebase is not considered yet
> > I'm waiting feedback about that before touching
> > the rebase code.
>
> I encourage you to read back in the history of the list as to why we
> haven't done this and why it's not likely to be accepted now, but let me
> provide a few reasons of my own.
>
> First, as any contributor to the mailing list can tell you, I am
> absolutely terrible at naming things.  I frequently name my branches
> something that makes sense to me at the time without regard to whether
> that will make sense in the future.  I don't want to memorialize my
> momentary thoughtlessness in the history of the repository forever.
>

I think you got the point! Git is written by software people for
software people and we know that we don't like to keep track of our
mistakes... Git allows you to to work like this because you can have
your own local branches with the name you want, just use a git merge
--no-ff at the end and only the commits that make senses will be in
the repository forever.
For CMM compliance, the branch type and the branch name must be
described in a SCM plan. In big open source project, I understand it
is not the so important because, at the end, we will only integrate
one patch on the master trunk, it is not like this for most of the
projects in companies.

> Second, one workflow I commonly use is creating a branch with many
> commits and then breaking them down into small series that are logical
> and easy for review.  If I have a branch called "test-fixes-part7" with
> 50 commits and then I decide to split that into two branches,
> test-fixes-part7 and test-fixes-part8, by copying the branch and using
> git reset --hard to truncate the old one, I don't want the old branch
> name in my new branch.  A lot of Git workflows assume you can reset and
> rename branches this way and having the branch name in the commit header
> breaks those workflows.

I understand but it will not break the workflow. .. the fact to have
the branchname as information in the commit header could be no more
than an "additional information". Exactly as you name and email is in
the commit header too It is simply an additional information for those
who want it, who need it. If you don't want to use is, that's right,
this information is even not visible in a normal git log command.It is
only visible with a dedicated placeholder.

>
> Third, people reuse branch names.  Right now, I have eight branches with
> test fixes all starting with "test-fixes-part" because I'm working on
> one major project with all of those test fixes.  However, if a developer
> working on another major project also has a lot of changes to the test
> suite, they may have lots of identically named branches, which would be
> confusing, since our identically named test fix branches would relate to
> different projects.  (See my first point.)

If it is different projects... it will not be confusing. And again,
you have 2 situations:
1. People working with SCM plan and hard branch naming convention. In
this case, they will never get this kind of problem, duplicated name
are impossible "by construction"
2. People that are working with temporary branch will not use the
branchname in the header and having duplicate branchname will no be
confusing.

>
> However, despite the fact that we aren't likely to add this in the
> commit header, there are definitely ways to achieve this.
>

I agree with the exception of the fact that it will depend on the
thoroughness of the developers themselves. And it will be more risky
to develop new features that has to be confident in the commit
message.

> If you want to include the branch name in the commit, you can do so with
> a trailer.  git interpret-trailers can then be used to manipulate and
> extract these, and along with a hook, add them automatically if they're
> missing.

Unfortunately, hooks are (can be) personal and there no way I know to
force a hook to be used. That exactly what I was using today but a lot
of developer that start on the project, clone it but does not use the
correct hook. So, finally, It is impossible to be confident in the
commit message content to get the name of the branch.

Let me explain our workflow (which is a really common one):
Everyday, for backup reasons, all developers have to push their work
on the central repository (at least their major development branch,
not the experimental). In this context, the rebase is not used (should
even be forbidden) but we are using the "merge back" instead (merge
master to localdev) very often to guarantee the coherence between the
current master state and the development..
At the end of each "small development" (bug fix or feature request),
the branch is merged with no fast-forward.
So we have this kind of graph:

*   d122671 (HEAD -> master) master: Merge branch 'dev_feature1'
|\
| * 7a6a93d (dev_feature1) dev_feauture1: some new changes
| *   9e498de Merge branch 'master' into dev_feature1
| |\
| |/
|/|
* | 2fca855 master: feature x merged
| * 3610279 dev_feauture1: some other changes
| * ae69fa8 dev_feauture1: some changes
|/
* 8bfee18 master:current status

As you can see, to explain the flow I had to add the branchname on
each commit ;-)

So, each new version in the master branch is the result of merge
(exactly what you are doing when you integrate patch) but with
difference that we want to keep track of feature history and we want
to be confident in the tool, not in the rigor of our developers. And
we don't want to see the development commits as history of the master
version:
To be more clear, the master previous version is 2fca, not 7a6a nor 9e49.
And it is important to have that view to answer a lot of question
(why, when, what, who, which effort, how long, how many, ... )

>
> If you're working on a more centralized project and you want to require
> the branch name in your commit trailers, you can set your CI system to
> fail or reject commits that don't contain them.  This is the approach
> that systems like Gerrit use when the required trailers are missing and
> it seems to work reasonably well.

I don't know gerrit but I think it is an something that use git as
kernel but I don't know if developers have directly access to the git
repository. or has to use the web interface.
of course, with a upper layer, everything is still possible.

>
> Hopefully these suggestions are helpful in getting you the traceability
> you desire without requiring fundamental changes to the way Git works.

Again, what I propose is certainly not a fundamental change but just
an additional metadata to help those who need it.
I even propose to configure a variable to activate it (even if I
prefer it is activated by default to be sure it is done by new users).
I will publish the patch with the variable but I've seen a small bug
to fix before ;-)

Thanks for your feedback,

Arnaud

> --
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204

[RFC PATCH 0/2 v2] *** Add branchname in commit when core.branchnameincommit is set ***

[Arnaud Bertrand]

To avoid any change in the current git behaviour by default, I've added
a configuration variable that allow to activate the feature for those
who want to see the branchname in commit. 
By default, this feature is disabled

Arnaud Bertrand (2):
  Add branchname in commit header
  Add the configuration parameter core.branchnameincommit

 Documentation/pretty-formats.txt |  1 +
 cache.h                          |  1 +
 commit.c                         | 21 +++++++++++++++++++++
 config.c                         |  5 +++++
 environment.c                    |  1 +
 pretty.c                         | 15 +++++++++++++++
 6 files changed, 44 insertions(+)

-- 
2.25.0.rc0.7.g17b02bf28a

[RFC PATCH 1/2 v2] Add branchname in commit header

[Arnaud Bertrand]

Add the branchname in the commit header before the commit message
the following line is added:

branch <branchname>

where <branchname> comes from the function resolve_ref_unsafe("HEAD",...)
without the prefix refs/heads/
A placeholder is added to the pretty format "%Xb" to print the branch information,
X if for "extra-header" and can be use in the future for new features
b is of course for "branch"

the %Xb returns an empty string when branchname information is not found
---
 Documentation/pretty-formats.txt |  1 +
 commit.c                         | 11 +++++++++++
 pretty.c                         | 15 +++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 1a7212ce5a..bd52908f53 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -241,6 +241,7 @@ endif::git-rev-list[]
 '%gE':: reflog identity email (respecting .mailmap, see
 	linkgit:git-shortlog[1] or linkgit:git-blame[1])
 '%gs':: reflog subject
+'%Xb':: branchname in which commit was done
 '%(trailers[:options])':: display the trailers of the body as
 			  interpreted by
 			  linkgit:git-interpret-trailers[1]. The
diff --git a/commit.c b/commit.c
index 434ec030d6..f64a0698be 100644
--- a/commit.c
+++ b/commit.c
@@ -1425,6 +1425,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 	int result;
 	int encoding_is_utf8;
 	struct strbuf buffer;
+	const char *branch = "Unknown";
+	int flags;
+	const char *lbranch =resolve_ref_unsafe("HEAD",0,NULL,&flags);
 
 	assert_oid_type(tree, OBJ_TREE);
 
@@ -1453,6 +1456,14 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		author = git_author_info(IDENT_STRICT);
 	strbuf_addf(&buffer, "author %s\n", author);
 	strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT));
+	if (lbranch) {
+	   skip_prefix(lbranch,"refs/heads/",&branch);
+	   strbuf_addf(&buffer, "branch %s\n", branch);
+	}
+	else {
+	   strbuf_addf(&buffer, "branch Unknown\n");
+	}
+
 	if (!encoding_is_utf8)
 		strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
 
diff --git a/pretty.c b/pretty.c
index 305e903192..5961c39398 100644
--- a/pretty.c
+++ b/pretty.c
@@ -804,6 +804,7 @@ struct format_commit_context {
 
 	/* The following ones are relative to the result struct strbuf. */
 	size_t wrap_start;
+	char *branch;
 };
 
 static void parse_commit_header(struct format_commit_context *context)
@@ -1367,6 +1368,20 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
 		return 1;
 	}
 
+
+	/* Now add extra header info */
+	if (placeholder[0] == 'X') {
+		switch (placeholder[1]) {
+		case 'b': /* branch ... */
+			c->branch = get_header(msg,"branch");
+			if (c->branch)
+				strbuf_addstr(sb, c->branch);
+			free(c->branch);
+			return 2;
+		}
+	}
+
+
 	/* Now we need to parse the commit message. */
 	if (!c->commit_message_parsed)
 		parse_commit_message(c);
-- 
2.25.0.rc0.7.g17b02bf28a

[RFC PATCH 2/2 v2] Add the configuration parameter core.branchnameincommit

[Arnaud Bertrand]

With this parameter, which is 0 by default (no change compare
to the normal behaviour) you have the possibility to activate
this feature to have the branchname in the header commit
When it exists, the branchname is accesible in the git log
with the pretty format placehoder "%Xb".
---
 cache.h       |  1 +
 commit.c      | 24 +++++++++++++++++-------
 config.c      |  5 +++++
 environment.c |  1 +
 4 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index 1554488d66..dd7188a4e4 100644
--- a/cache.h
+++ b/cache.h
@@ -949,6 +949,7 @@ void reset_shared_repository(void);
  * commands that do not want replace references to be active.
  */
 extern int read_replace_refs;
+extern int branchname_in_commit;
 extern char *git_replace_ref_base;
 
 extern int fsync_object_files;
diff --git a/commit.c b/commit.c
index f64a0698be..e63d97d308 100644
--- a/commit.c
+++ b/commit.c
@@ -1428,6 +1428,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 	const char *branch = "Unknown";
 	int flags;
 	const char *lbranch =resolve_ref_unsafe("HEAD",0,NULL,&flags);
+	int flbranchinextra = 0;
 
 	assert_oid_type(tree, OBJ_TREE);
 
@@ -1456,21 +1457,30 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 		author = git_author_info(IDENT_STRICT);
 	strbuf_addf(&buffer, "author %s\n", author);
 	strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT));
-	if (lbranch) {
-	   skip_prefix(lbranch,"refs/heads/",&branch);
-	   strbuf_addf(&buffer, "branch %s\n", branch);
-	}
-	else {
-	   strbuf_addf(&buffer, "branch Unknown\n");
-	}
 
 	if (!encoding_is_utf8)
 		strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
 
 	while (extra) {
+		/* when commit is reworked - e.g. amend, the banch is already
+		 * in extra-header and must not be modified
+		*/
+		if (!strcmp(extra->key,"branch"))
+			flbranchinextra=1;
 		add_extra_header(&buffer, extra);
 		extra = extra->next;
 	}
+
+	if (branchname_in_commit && !flbranchinextra) {
+		if (lbranch) {
+			skip_prefix(lbranch,"refs/heads/",&branch);
+			strbuf_addf(&buffer, "branch %s\n", branch);
+		}
+		else {
+			strbuf_addf(&buffer, "branch Unknown\n");
+		}
+	}
+
 	strbuf_addch(&buffer, '\n');
 
 	/* And add the comment */
diff --git a/config.c b/config.c
index d75f88ca0c..bec1b5c3af 100644
--- a/config.c
+++ b/config.c
@@ -1389,6 +1389,11 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.branchnameincommit")) {
+		branchname_in_commit = git_config_bool(var, value);
+		return 0;
+	}
+
 	/* Add other config variables here and to Documentation/config.txt. */
 	return platform_core_config(var, value, cb);
 }
diff --git a/environment.c b/environment.c
index e72a02d0d5..1d266a91cf 100644
--- a/environment.c
+++ b/environment.c
@@ -52,6 +52,7 @@ const char *askpass_program;
 const char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
 int read_replace_refs = 1;
+int branchname_in_commit = 0;
 char *git_replace_ref_base;
 enum eol core_eol = EOL_UNSET;
 int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
-- 
2.25.0.rc0.7.g17b02bf28a