Allow git commit hashes to be abbreviated to 7 characters

[olly@…]

Currently an abbreviated git commit hash is only recognised by trac if it's at least 8 characters. But git itself defaults to abbreviating to 7 characters (if the 7 character abbreviation is unique in the repo) - to demonstrate this:

olly@pippikin:~/git/xapian$ git log -1 --oneline
575eab1 docs/: Add a document about character encoding, as suggested by James Aylett in #550.

The fix is a single character change - I'll attach a patch in a moment.

This seems to affect 1.0.4 and trunk.

[olly@…]

proposed fix

[olly@…]

BTW, we've been using this patch on ​http://trac.xapian.org/ for a few months, and it's working well for us.

[Peter Suter]

​How short can Git abbreviate? has an interesting overview. 7 is indeed the default, but can give duplicates fairly quickly. (Linux has thousands and even some at 11.)

Trac has one too:
changeset:7c28acd0081d09d2b951a82518ad0ca132d1dd02/psuter.git changeset:7c28acd58bc4954755ec43807f5359ec92b0674d/psuter.git

Xapian:
​http://trac.xapian.org/changeset/f6d3c7cc9277b0163d5c7d0d3ce29d483c8bea5d/git
​http://trac.xapian.org/changeset/f6d3c7c6f46af185c5fd7cc875e642e5424a2b72/git

[Peter Suter]

Hm, the Git plugin has two config options​ shortrev_len (defaults to 7) and wiki_shortrev_len (defaults to 40).

[Olly Betts <olly@…>]

I appreciate that abbreviated hashes can be (and indeed can become) ambiguous, and that a longer minimum length reduces the likelihood, but I think trac's default really should match git's.

Are you suggesting it using shortrev_len here rather than hard-coding a length?

[Peter Suter]

Replying to Olly Betts <olly@…>:

I appreciate that abbreviated hashes can be (and indeed can become) ambiguous, and that a longer minimum length reduces the likelihood, but I think trac's default really should match git's.

I can see the logic of matching Git's default. This is not Git specific code though. Mercurial typically ​uses 12 characters.

Are you suggesting it using shortrev_len here rather than hard-coding a length?

I think [git] wiki_shortrev_len would be the relevant one for this case. Have you tried just changing that to 7 in your trac.ini?

This code was changed in #4261 from 6 to 8 to avoid false matches. I'm not sure what the best solution is. It does seem a bit weird that the Git plugin basically reimplements this and that the core functionality is not configurable.

[Ryan J Ollos]

Replying to psuter:

Replying to Olly Betts <olly@…>: I can see the logic of matching Git's default. This is not Git specific code though. Mercurial typically ​uses 12 characters.

In that case, should the functionality be moved from the GitPlugin to the versioncontrol package?

Are you suggesting it using shortrev_len here rather than hard-coding a length?

I think [git] wiki_shortrev_len would be the relevant one for this case. Have you tried just changing that to 7 in your trac.ini?

Make sure to use wikishortrev_len as the option name. wiki_shortrev_len would be a better choice though - we should consider renaming it.

wikishortrev_len is 40 for this Trac instance. I can't figure out what is going on here:

• [7c28acd/psuter.git]
• [7c28acd0/psuter.git]
• 7c28acd0/psuter.git
• 7c28acd0081/psuter.git
• 7c28acd0081d/psuter.git
• 7c28acd0081d09d2b951a82518ad0ca132d1dd02/psuter.git

I vaguely remember being confused about this in the past and discussing somewhere, but I can't locate any conversation at the moment.

[Peter Suter]

(Another false match with 6 characters.)

Replying to rjollos:

Make sure to use wikishortrev_len

Oops, thanks for noticing.

Replying to psuter:

Mercurial typically ​uses 12 characters.

Hm, the mercurial-plugin actually also has its own competing functionality​ with 12+ characters, explaining this:

• 7c28acd0081/psuter.git
• 7c28acd0081d/psuter.git

While the generic​ one only matches [...] wrapped links (with 8+ characters):

• [7c28acd/psuter.git]
• [7c28acd0/psuter.git]

So the git plugin was not involved at all here.

In that case, should the functionality be moved from the GitPlugin to the versioncontrol package?

Possibly, yes. The current situation seems a bit confusing for sure.

[Peter Suter]

#12760 was closed as a duplicate.

[strk@…]

I'm also having this problem, shortrev_len is not being honoured when set to 7 (which should be also the default, according to TracIni#git-shortrev_len-option. It's weird because using changeset:<short-hash>/git works great instead, so there's an asymmetricity between changeset:... and [...]

[Jun Omae]

Replying to Ryan J Ollos:

In that case, should the functionality be moved from the GitPlugin to the versioncontrol package?

Sounds good.

Another issue, the hex-string wiki syntax in tracopt.v.git.git_fs works only for default repository. The hex-string wiki syntax in mercurial-plugin has same issue. Changeset id syntax in t.v.web_ui.changeset works for both default repository and non-default repository (r1 for default, r1/reponame for non-default).

The proposed patch is okay to me.

Since default of [git] shortrev_len is 7, even though I try to paste hex-string (7 chars) from changeset view between [ and ] in wiki, it doesn't work. It would annoy me.

[Ryan J Ollos]

Proposed changes to try and clarify the documentation (for trunk):

tracopt/versioncontrol/git/git_fs.py

@@ -272 +272 @@
         """Wrap `GitRepository` in `CachedRepository`.""")
 
     shortrev_len = IntOption('git', 'shortrev_len', 7,
-        """The length to which a sha1 will be abbreviated to (must
-        be >= 4 and <= 40).
+        """The length at which a sha1 is abbreviated (must be >= 4 
+        and <= 40).
         """)
 
     wiki_shortrev_len = IntOption('git', 'wikishortrev_len', 40,
-        """The minimum length for which a hex-string is formatted as
-        a changeset TracLink (must be >= 4 and <= 40).
+        """The minimum length for which a hex-string in wiki content
+        is formatted as a changeset TracLink (must be >= 4 and <= 40).
         """)
 
     trac_user_rlookup = BoolOption('git', 'trac_user_rlookup', 'false',1

[Ryan J Ollos]

comment:12 change committed to trunk in r16096.

[ebouaziz@…]

About this option and the feature it is related to,

it also collides with the plantuml plugin: the hash in the name of the generated image is replaced in the page source (with a message saying the changeset does not exists in the repo) and the image is not displayed.

[Ryan J Ollos]

Replying to Jun Omae:

Since default of [git] shortrev_len is 7, even though I try to paste hex-string (7 chars) from changeset view between [ and ] in wiki, it doesn't work. It would annoy me.

I've also experienced this frustration when copying a hash from the changeset page. For the interim, I've changed [git] shortrev_len to 8 for this site.