Github can have troubles with compare URLs using abbreviated hashes

[FMCorz]

A safe bet would be to either:

• Use full hashes
• Use hashes of 10+ characters

[danpoltawski]

There doesn't seem to be a way to create an issue with github, only raise a support request - I will try just to see if we get a response.

[danpoltawski]

I wrote to them:

We have noticed that one of our commits in the moodle/moodle.git
repository results in a 404 if using a short 7 char sha syntax. As far
as we can tell this commit does not have a clashing sha and should
display. The problem appears to be specific to this commit.

The commit in question is:

https://github.com/moodle/moodle/commit/d230899db89b3c1539bdd1be3f3f776cfe3a85f4

The 7 char shortned syntax does not work:

https://github.com/moodle/moodle/commit/d23089

However, when using local git tools, this commit does display and
doesn't appear to be clashing (and is generated by the %h: abbreviated
commit hash commit formating option):

$ git log --format="%h %s" origin/master  -n1
d230899 weekly release 3.0dev

$ git show d230899
commit d230899db89b3c1539bdd1be3f3f776cfe3a85f4
Author: Eloy Lafuente (stronk7) <stronk7@moodle.org>
Date:   Thu Sep 17 18:01:12 2015 +0200

    weekly release 3.0dev

[snip]

Furthermore, on the commit page (using full sha), the sha is displayed
in that shortneed syntax at the bottom:

"0 comments on commit d230899"

The problem also affects the diff url tool (where it was detected).

[danpoltawski]

I got a response from github support :+1:

Hi Dan,

Thanks for reaching out. I'm not seeing a clash when I check out a fresh clone, but I do see a clash when I run this on our side: $ git rev-parse --disambiguate=d23089

d230899220c4a3f590cb75c5693467cb01950a45 d230899db89b3c1539bdd1be3f3f776cfe3a85f4

One of those is a blob, and one of those is a commit. I'm guessing that the reason why we're not seeing the same in a local clone is that the commit containing the blob is no longer reachable (e.g. it was force-pushed away), so it's not included when cloning.

I've asked the team to look into this -- hopefully there's a way to fetch the right commit in this case and ignore the blob, which I think is causing problems currently. I can't promise anything, but I'll get back to you as soon as there's any news.

I also wanted to ask where exactly on GitHub you found links which pointed to that page which returns a 404. In other words, where did you find a link for this (or the broken diff URL you mentioned):

https://github.com/moodle/moodle/commit/d23089

Thanks!

Ivan

[danpoltawski]

On Mon, Sep 21, 2015 at 1:57 PM, Ivan Žužak support@github.com wrote:

Thanks for explaining. Yeah, switching to full SHAs would have been my recommendation if you hadn't suggested it already -- that should keep you out of problems. :)

I'll also followup when there's news about this on our end.

Cheers, Ivan

On Mon, Sep 21, 2015 at 1:53 PM, Dan Poltawski dan@moodle.com wrote:

Hi Ivan,

Thanks for getting back to me, I had a suspicion it could be something like that.

I also wanted to ask where exactly on GitHub you found links which pointed to that page which returns a 404. In other words, where did you find a link for this (or the broken diff URL you mentioned):

Ah, we did not find that URL through the github interface. We were relying on your cool support of that URL structure to generate links in another tool and found it creating an mistakenly empty diff url, e.g: https://github.com/abgreeve/moodle/compare/d230899...wip-MDL-50917-master

We will switch to using full sha hashes in those urls now - https://github.com/FMCorz/mdk/issues/120 :)

thanks again for the prompt response!

Dan

[danpoltawski]

Just encountered this issue again - is there a fix?

[FMCorz]

In mdk/moodle.py, change the following:

headcommit = self.git().hashes(ref=ref, limit=1, format='%h')[0]
to
headcommit = self.git().hashes(ref=ref, limit=1, format='%H')[0]

That should work.

[andrewnicols]

And again with this week's weekly. Are you accepting pull requests for this?

[FMCorz]

Yes, someone has submitted one I think, but I didn't like the long hashes, I think we should keep them to a reasonable minimum.

[danpoltawski]

Maybe something counting the number of commits and using the ~N syntax would satisfy your needs? It does have the advantage of minimising updates need to be done to the tracker

i.e. https://github.com/danpoltawski/moodle/compare/MDL-54822-master~1...MDL-54822-master

(though I would prefer the full hash I think)