On Tue, Jan 29, 2019 at 7:07 PM Ferruh Yigit [off-list ref] wrote:

On 1/29/2019 5:34 PM, David Marchand wrote:

quoted

On Tue, Jan 29, 2019 at 4:31 PM Ferruh Yigit [off-list ref]

wrote:

quoted

quoted

Fixes line commit id length defined as 12 in fixline alias:
fixline = log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'

Check if the Fixes line commit id length matches the defined value.

Can't git decide to report a longer string in case of collisions of
abbreviated id ?

Tried this for 2 characters, and git forcefully reported 5 chars:
$ git log -1 --abbrev=2 origin/master --format='Fixes: %h (\"%s\")'
Fixes: a2f9c (\"version: 19.02-rc4\")

I did not find any collisions with 12 characters abbreviated commitid,

but

quoted

I am not sure enforcing the check on exactly 12 characters is a good idea
in the long run.

Yes git can report a longer string in case of collisions, but I don't
expect to
have one with 12 characters.

This is mainly for some cases either people use full 40 chars or small
ones.

Indeed in background I am (and most probably Thomas too) fixing them while
merging, I thought it is better idea to integrate that into script so that
developers can be aware of the syntax issue and fix it before sending.

I can understand you want to avoid such edits yes, and so this patch.

However, I think we can do one more thing.
The contributing guide does indicate you are supposed to run both
checkpatches.sh and check-git-log.sh.
I am pretty sure I missed this second step in the past..

How about calling check-git-log.sh from checkpatches.sh ?
check-git-log.sh does not support patch files as input, so it would need
support for it.

quoted

quoted

Signed-off-by: Ferruh Yigit <redacted>
---
Cc: Qi Zhang <redacted>
---
 devtools/check-git-log.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index d39064f9d..f4d6c1fba 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh

@@ -177,6 +177,11 @@ bad=$(for fixtag in $fixtags ; do
 done | sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"

+bad=$(for fixtag in $fixtags ; do

+       echo $fixtag | awk '{print $2}' | awk 'length != 12 {print}'

quoted

+done)

quoted

Not an awk expert (this could be done in pure shell, but this is a
different story :-p), but I would see something like:

for fixtag in $fixtags; do
  echo $fixtag | awk 'length($2) < 12 { print $2 }';
done

Yes, looks better, I will update the script.

And no specific preference on shell or awk implementation, there is no
performance concern in this script and awk already used by it, I am good
as long
as it is functional.

Well, I've seen Thomas reply, looks even better ;-).


-- 
David Marchand