Defect #3724

Mercurial repositories display revision ID instead of changeset ID

Added by Scot Herring over 8 years ago. Updated almost 7 years ago.

Status:ClosedStart date:2009-08-10
Priority:LowDue date:
Assignee:Toshi MARUYAMA% Done:

0%

Category:SCM
Target version:1.2.0
Resolution:Fixed Affected version:

Description

Redmine exposes Mercurial's revision IDs (one-up decimal number, similar to Subversion) as the primary means of referring to changesets, rather than the changeset ID (strings of hex digits, like Git). While this does work, it's explicitly discouraged in Mercurial's documentation.

From the FAQ:

You should always use some form of changeset ID rather than the local revision number when discussing revisions with other Mercurial users as they may have different revision numbering on their system.

And the page on revision numbers says:

Pitfalls

Revision numbers referring to changesets are very likely to be different in another copy of a repository. Do not use them to talk about changesets with other people. Use the changeset ID instead.

(Emphasis theirs.)

The behavior of hgweb.cgi (the web frontend that comes with Mercurial) is to expose only the short-form changeset IDs. However it allows entering a revision ID into the URL as well. So the following URLs will bring up the same patch:

This is probably the sanest path to take with Redmine, in terms of keeping old URLs functioning and also to more or less parallel the way that Git repos are managed.

Issue #3567 might be tangentially related to this.

mercurial_scmid_authoritative.patch Magnifier - Make scmid the authoritative revision, since revisions are ambiguous to a DVCS (2.89 KB) Peter Fern, 2009-12-15 14:44

hg-overhaul-000.png (8.77 KB) Toshi MARUYAMA, 2010-03-13 06:05

hg-overhaul-001.png (11.4 KB) Toshi MARUYAMA, 2010-03-13 06:05

hg-overhaul-002.png (13.7 KB) Toshi MARUYAMA, 2010-03-13 06:05

hg-overhaul-003.png (25 KB) Toshi MARUYAMA, 2010-03-13 06:05

before-link.png (53.9 KB) Toshi MARUYAMA, 2011-01-25 12:32

before-repository.png (105 KB) Toshi MARUYAMA, 2011-01-25 12:32

after-annotate.png (81.4 KB) Toshi MARUYAMA, 2011-01-25 12:32

after-link-id.png (55.7 KB) Toshi MARUYAMA, 2011-01-25 12:32

after-link-revno.png (54.5 KB) Toshi MARUYAMA, 2011-01-25 12:32

after-repository.png (93.3 KB) Toshi MARUYAMA, 2011-01-25 12:32

before-annotate.png (68.9 KB) Toshi MARUYAMA, 2011-01-25 12:32


Related issues

Related to Redmine - Feature #6092: Truncate Git revision labels in Activity page/feed and al... Closed 2010-08-09
Related to Redmine - Defect #4924: mercurial reader disregard information from non-default b... Closed 2010-02-24
Related to Redmine - Defect #6681: Mercurial, Bazaar and Darcs auto close issue text should ... Closed 2010-10-15
Related to Redmine - Feature #4455: Mercurial overhaul Closed 2009-12-21
Related to Redmine - Feature #1273: Need a way to re-sync repository history New 2008-05-21
Related to Redmine - Defect #7518: Mercurial diff can be wrong if the previous changeset isn... Closed 2011-02-02
Related to Redmine - Defect #8030: Bazaar integration doesn't notice new commits to repository New 2011-03-31
Related to Redmine - Feature #5501: Git: Mercurial: Adding visual merge/branch history to rep... Closed 2010-05-11

Associated revisions

Revision 4613
Added by Toshi MARUYAMA almost 7 years ago

Changing revision label and identifier at SCM adapter level (#3724, #6092)

Contributed by Yuya Nishihara.

Revision 4654
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: accept both of revision and nodeid as changeset id (#3724).

Contributed by Yuya Nishihara.

Revision 4661
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: cat test for accept both of revision number and changeset id (#3724).

Revision 4662
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: fix diff and test for accept both of revision number and changeset id (#3724).

Diff of changeset can be wrong if the previous changeset isn't the parent.

Revision 4663
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: annotate accepts both of revision number and changeset id (#3724).

Change annotate revision label to Mercurial style '4:def6d2f1254a'
and use identifier.

Revision 4664
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: functional test for accept both of revision number and changeset id (#3724).

Revision 4686
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: diff '-c' option supports above Mercurial 1.2 (#3724, #7253).

r4662 fails in old Mercurial.

Revision 4694
Added by Toshi MARUYAMA almost 7 years ago

scm: use scmid for "commit:xxx" link if available (#3724).

scmid is more solid than revision number.

Contributed by Yuya Nishihara.

Revision 4695
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: use scmid as identifier (#3724).

Contributed by Yuya Nishihara.

Revision 4696
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: add "commit:xxx" and "rNN" link test for mercurial (#3724).

Revision 4697
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: use revision text mercurial style "2:400bb8672109" (#3724).

Contributed by Yuya Nishihara.

Revision 4698
Added by Toshi MARUYAMA almost 7 years ago

scm: changing two revision diff text at SCM adapter level (#3724).

Revision 4699
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: change two revision diff text of mercurial (#3724).

For example, "4:def6d2f1254a 2:400bb8672109".

Revision 4705
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: unit lib test for entries accept both of revision number and changeset id (#3724, #3421)

Revision 4733
Added by Toshi MARUYAMA almost 7 years ago

scm: use format_revision() for history, view and annotate (#3724).

Revision 4734
Added by Toshi MARUYAMA almost 7 years ago

scm: functional test of using format_revision() for annotate (#3724).

Revision 4857
Added by Toshi MARUYAMA almost 7 years ago

scm: mercurial: add identifier to entry.lastrev (#3724, #3421).

Revision 4858
Added by Toshi MARUYAMA almost 7 years ago

scm: in repository tree, use find_changeset_by_name instead of changesets.find_by_revision (#3724, #3421).

Mercurial revision numbers are far too brittle.
Please see #6681 description.

History

#1 Updated by Luke Hoersten about 8 years ago

Yeah this is a problem for us too. It's very fragile and something as simple as removing a changeset with screw up ALL the links. This really needs to be fixed.

#2 Updated by Peter Fern almost 8 years ago

The attached patch addresses your concern, and I'm all for moving away from the ambiguous revision numbers, but this sort of behavioural change probably requires a configuration option, since it will definitely break any existing r##-style references... comments?

#4 Updated by Peter Fern almost 8 years ago

Actually, there are some further changes to be made in app/models/repository/mercurial.rb

#5 Updated by Luke Hoersten almost 8 years ago

Peter Fern wrote:

The attached patch addresses your concern, and I'm all for moving away from the ambiguous revision numbers, but this sort of behavioural change probably requires a configuration option, since it will definitely break any existing r##-style references... comments?

Probably a conversion script of the r##-style references to hashes would be best. Thanks for doing this!

#6 Updated by Peter Fern almost 8 years ago

Just an update - I have a significant Mercurial patch near completion that addresses this issue, plus a number of others, please ignore my previous attachment.

#7 Updated by Scot Herring almost 8 years ago

Awesome. Look forward to it.

I don't see why it'd break r##, though - like I mentioned above, hg understands several kinds of references to changesets, so what's preventing redmine from allowing all three and just passing along whatever identifier to the client?

#8 Updated by Peter Fern almost 8 years ago

There are two keys in the Redmine database - revision and scmid, however Mercurial has 3 identifiers: revision, short changeset ID, and full changeset ID. Since revision is likely to get clobbered, and even the short changeset ID is not guaranteed to remain unique, I'd like to move to using the short ID as the revision, and the full ID as the scmid, since this is the only way we'll have a guaranteed unique identifier in the DB, and if the revision number gets routinely clobbered by merges, it's pretty useless anyway, since any references to it will now refer to some other commit.

#9 Updated by Luke Hoersten almost 8 years ago

Peter Fern wrote:

Just an update - I have a significant Mercurial patch near completion that addresses this issue, plus a number of others, please ignore my previous attachment.

If you're working on a big Mercurial update, you may want to take a look at issue r3421. Mercurial is more or less useless without it in Redmine.

#10 Updated by Luke Hoersten almost 8 years ago

Luke Hoersten wrote:

Peter Fern wrote:

Just an update - I have a significant Mercurial patch near completion that addresses this issue, plus a number of others, please ignore my previous attachment.

If you're working on a big Mercurial update, you may want to take a look at issue r3421. Mercurial is more or less useless without it in Redmine.

Sorry that's issue #3421.

#11 Updated by Peter Fern almost 8 years ago

I wouldn't say that - the only thing that doesn't work is file sizes... my patch incorporates branch and tag support like git, plus major performance improvements. I'll have it up in the next day or so. I've added a comment to #3421

#12 Updated by Luke Hoersten almost 8 years ago

Peter Fern wrote:

I wouldn't say that - the only thing that doesn't work is file sizes... my patch incorporates branch and tag support like git, plus major performance improvements. I'll have it up in the next day or so. I've added a comment to #3421

You understand what #3421 is fixing right? Redmine pulls the history from the repo but file access is attempted on the file system instead of using 'hg cat' to grab what's in the changeset specified.

#13 Updated by Peter Fern almost 8 years ago

I think you'll find that's incorrect, and looking at the patch, it doesn't modify the 'cat' method in any way other than to add a default identifier of 'tip'. The only file access the the Mercurial code is to get file sizes, which I'd be happy to see resolved since it's the last functional hole in my patchset.

#14 Updated by Ted Lilley over 7 years ago

As a user of the Mercurial plugin, I want to add my $.02 here.

Just as there is a reason for the globally-unique changeset hash id, there is also a reason for the integer revision id as well. I currently rely on Redmine as the source for access to the integer id, and removing it completely would break my build system implementation.

While a changeset hash is useful when discussing revisions between developers, it is unfortunately both completely unsuitable for use in build numbering of packages as well as being hostile to the human eye. Because of the necessity of numbering our builds with the revision number, in order to trace back any build package to the source code which created it, we simply make sure that our developers know of the mutability of the integer revision id and use the build repository as its authoritative source. This is simply the added cost of using a system like Mercurial, that you have to keep in mind something extra like the scope of the revision id.

It's not simply a matter of saying "suck it up, use the hash", either. When building .NET assemblies, the revision number must fit within a 16-bit integer. A hash can't possibly do this. Even if it were possible, however, I still wouldn't inflict hash numbers on our customers as a means of identifying a package.

So, whatever change you make, I would ask that both it preserve the integer revision id, as well as allowing r-style links in Redmine and being available in both the Repository view as well as the atom feed for the repository. Otherwise, Redmine's mercurial functionality will be significantly less useful to developers like me.

#15 Updated by Luke Hoersten over 7 years ago

From the Mercurial book :

[...] the same number in two different clones of a repository may identify different changesets.

What that means is if you swap out the repo for a modified clone, potentially all your short numbers which you've "hardcoded" into the Redmine issues/comments would be invalidated.

I want my Redmine comments to refer to a specific changeset, not a specific clone's changeset. Perhaps if you're using Mercurial in a more centralized manor this is fine, but my team doesn't work that way.

Ted Lilley wrote:

As a user of the Mercurial plugin, I want to add my $.02 here.

Just as there is a reason for the globally-unique changeset hash id, there is also a reason for the integer revision id as well. I currently rely on Redmine as the source for access to the integer id, and removing it completely would break my build system implementation.

While a changeset hash is useful when discussing revisions between developers, it is unfortunately both completely unsuitable for use in build numbering of packages as well as being hostile to the human eye. Because of the necessity of numbering our builds with the revision number, in order to trace back any build package to the source code which created it, we simply make sure that our developers know of the mutability of the integer revision id and use the build repository as its authoritative source. This is simply the added cost of using a system like Mercurial, that you have to keep in mind something extra like the scope of the revision id.

It's not simply a matter of saying "suck it up, use the hash", either. When building .NET assemblies, the revision number must fit within a 16-bit integer. A hash can't possibly do this. Even if it were possible, however, I still wouldn't inflict hash numbers on our customers as a means of identifying a package.

So, whatever change you make, I would ask that both it preserve the integer revision id, as well as allowing r-style links in Redmine and being available in both the Repository view as well as the atom feed for the repository. Otherwise, Redmine's mercurial functionality will be significantly less useful to developers like me.

#16 Updated by Toshi MARUYAMA over 7 years ago

  • % Done changed from 0 to 80

Please see #4455 - Mercurial overhaul.

#17 Updated by Toshi MARUYAMA over 7 years ago

Luke Hoersten wrote:

From the Mercurial book :

[...] the same number in two different clones of a repository may identify different changesets.

What that means is if you swap out the repo for a modified clone, potentially all your short numbers which you've "hardcoded" into the Redmine issues/comments would be invalidated.

This is a very serious problem. But there is no way to resolve this problem simply. Because scmid of changeset table is not unique and revision is unique.

#18 Updated by Toshi MARUYAMA over 7 years ago

Luke Hoersten wrote:

What that means is if you swap out the repo for a modified clone, potentially all your short numbers which you've "hardcoded" into the Redmine issues/comments would be invalidated.

1

  /- B
A--------C

DB     hg
0 A    0 A
1 B    1 B
2 C    2 C

2

A---C

DB     hg
0 A    0 A
1 B    1 C
2 C

3

  /-----D
A---C

DB     hg
0 A    0 A
1 B    1 C
2 C    2 D

4

   /--------B
  /-----D
A---C

DB     hg
0 A    0 A
1 B    1 C
2 C    2 D
3 B    3 B

#19 Updated by Toshi MARUYAMA over 7 years ago

I finish testing of last big feature of #4455.
This feature is "old rNN record on DB and wikis can not delete".
http://github.com/marutosi/redmine/tree/hg-overhaul-0.9
http://github.com/marutosi/redmine/commit/30845a8c8ed51a71beb6e1d4df4dfbcf3a55c444

#20 Updated by Ammler _ over 7 years ago

I really hope, Mercurial will at least keep the option to use the revision number. That is in fact one of the big advantages compared to git, I am aware of the possibility to have same repo with different orders of changesets, but that doesn't matter, if you define server/buildsystem repo as master repo.

Please keep at least revision number for Mercurial as option.

I also changed hgweb to show revision numbers instead short hashes.

#21 Updated by Toshi MARUYAMA over 7 years ago

Thank you for your feedback.
Yuya's #4455 new implementation keeps redmine original DB structure.
Please see http://www.redmine.org/issues/4455#note-78 .

#22 Updated by Toshi MARUYAMA almost 7 years ago

  • Status changed from New to 7
  • Assignee set to Toshi MARUYAMA
  • Target version set to 1.2.0
  • % Done changed from 90 to 0

#23 Updated by Yuya Nishihara almost 7 years ago

To Toshi: JFTR, the arguments of assert_equal are expected, actual

r4654 trunk/test/unit/repository_mercurial_test.rb

assert_equal @repository.find_changeset_by_name(r).revision, '2'

should be

assert_equal '2', @repository.find_changeset_by_name(r).revision

#24 Updated by Toshi MARUYAMA almost 7 years ago

Yuya Nishihara wrote:

To Toshi: JFTR, the arguments of assert_equal are expected, actual

r4654 trunk/test/unit/repository_mercurial_test.rb

[...]

should be

[...]

I fixed and committed. Thanks.

#25 Updated by Toshi MARUYAMA almost 7 years ago

I think there are two cases of revision number inconsistency.

  • History editing (hg strip, hg rebase etc.)
  • Replacing repository

If you did history editing, you need to delete repository from redmine and re-add it.
There are some feature issues of re-syncing #1273.

About replacing repository, I wrote at #6681 description.

I think Mercurial user should not use "rNN" in wiki.
If someone wrote "rNN" in wiki, it is user's own risk.
#6681 changed auto close issue text from "rNN" to "commit:XXX".

Redmine stores revision number and changeset id in database.
To resolve this issue, we changed as following.

  • Change revision text to Mercurial style "2:400bb8672109"
  • Change revision link from revision number to changeset id
  • Redmine URL accepts both of revision number and changeset id
  • diff, cat and annotate accepts both of revision number and changeset id

Before



After




Also available in: Atom PDF