Defect #3421

Mercurial reads files from working dir instead of changesets

Added by Luke Hoersten over 12 years ago. Updated almost 11 years ago.

Status:ClosedStart date:2009-05-26
Priority:NormalDue date:
Assignee:Toshi MARUYAMA% Done:

0%

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

Description

Currently the Mercurial plugin tries to read the file contents from the working directory (similar to the "staging area" in Git) instead of the changesets themselves. This means when you try to view a file that's been removed from whatever changeset is checked out in the working dir, it will be not found. Or, trying to view a certain version of a file that does exist in the working dir will just display whatever version was checked out. This seems to be undesirable behavior. It'd be nice to use a null working directory similar to Git's "bare" staging area.

redmine-mercurial.patch Magnifier - Mercurial fix c.o. Ian Cardenas (3.78 KB) Luke Hoersten, 2009-06-09 15:33

size.py Magnifier - Mercurial size extension c.o. Ian Cardenas (1.33 KB) Luke Hoersten, 2009-12-19 04:20

branch00.png (9.35 KB) Toshi MARUYAMA, 2010-03-11 11:30

branch01.png (8.79 KB) Toshi MARUYAMA, 2010-03-11 11:30

Related issues

Related to Redmine - Feature #4455: Mercurial overhaul Closed 2009-12-21
Duplicated by Redmine - Defect #6458: Mercurial View just a directory browser Closed 2010-09-21

Associated revisions

Revision 4629
Added by Toshi MARUYAMA about 11 years ago

scm: mercurial: refactor mercurial unit test.

  • lib unit test switches if repository exists.
  • move cat test from app to lib.
  • no need to run "hg update" in unit test (#3421).

Revision 4705
Added by Toshi MARUYAMA about 11 years ago

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

Revision 4856
Added by Toshi MARUYAMA almost 11 years ago

scm: mercurial: rewrite MercurialAdapter#entries to show per-file change log and size (#3421, #4455).

Contributed by Yuya Nishihara.

Revision 4857
Added by Toshi MARUYAMA almost 11 years ago

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

Revision 4858
Added by Toshi MARUYAMA almost 11 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.

Revision 4864
Added by Toshi MARUYAMA almost 11 years ago

scm: mercurial: unit lib test for entries (#3421).

History

#1 Updated by Luke Hoersten over 12 years ago

  • Assignee set to Jean-Philippe Lang
  • % Done changed from 0 to 10

A friend is working on a patch for me. Will submit it when we have it.

#2 Updated by Luke Hoersten over 12 years ago

That'd be great if we could get this patch reviewed and applied. Currently, Mercurial support is nothing more than reading the contents out of some directory without this change. The change actually reads the repository itself.

#3 Updated by Jean-Philippe Lang over 12 years ago

I'm not sure to understand the actual defect. Please include unit tests in your patch.

#4 Updated by Anil Gulecha over 12 years ago

I see the same issue.. I pointed the repository for a project to a local clone of the mercurial repository.. and clicking on Repository takes a long while (It's been about half an hour, and the repository page still wont open). This is the opensolaris kernel repository. If there's any information I can provide about the setup, let me know.

#5 Updated by Luke Hoersten over 12 years ago

Jean-Philippe Lang wrote:

I'm not sure to understand the actual defect. Please include unit tests in your patch.

Do you understand what the staging area is in Git and why that is not the same thing as the repository? It's similar to that in Mercurial with the working director. I'm not sure what unit tests you are looking for. It won't show anything without a Mercurial repo attached to it.

#6 Updated by Jean-Philippe Lang over 12 years ago

Luke Hoersten wrote:

I'm not sure what unit tests you are looking for.

A test that would fail with the actual code and pass with your patch.

It won't show anything without a Mercurial repo attached to it.

Indeed. That's why there are test repositories (see doc/RUNNING_TESTS)

#7 Updated by Peter Fern about 12 years ago

Umm, where are you getting this 'hg size' command from?? It doesn't exist on any version I have access to...

#8 Updated by Luke Hoersten about 12 years ago

Peter Fern wrote:

Umm, where are you getting this 'hg size' command from?? It doesn't exist on any version I have access to...

Ian (the guy who wrote the patch) wrote a Mercurial plugin to give size. If it's not there it should just show '-' I think.

#9 Updated by Peter Fern about 12 years ago

Can you provide the mercurial plugin here?

#10 Updated by Luke Hoersten about 12 years ago

#11 Updated by Luke Hoersten about 12 years ago

Peter Fern wrote:

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'...

Add a file to your Mercurial, commit, then delete and commit. If you try to view that file through Redmine, it will say it's not there because it's trying to pull it from your working directory instead of the changeset itself.

#12 Updated by Peter Fern about 12 years ago

In the mean time, I've added the size method, and a conditional in the model for my upcoming patchset to try the scm.size method and fall back to accessing the filesystem if that returns nil.

#13 Updated by Peter Fern about 12 years ago

Luke Hoersten wrote:

Add a file to your Mercurial, commit, then delete and commit. If you try to view that file through Redmine, it will say it's not there because it's trying to pull it from your working directory instead of the changeset itself.

That wasn't happening here, but there were some errors handling revisions, etc - the 'entry does not exist in the repository' error is generic, so it's likely some other error was causing your problem, since the cat code currently in trunk works as expected.

Thanks for uploading the size extension, that's working great, and once I do some more testing, I'll get my patch up and link it to this issue for you to test.

If you're still in touch with Ian, can you ask him why he used "glob:#{path}**" instead of "path:#{path}" in the 'entries' method for the patch attached here?

#14 Updated by Peter Fern about 12 years ago

Peter Fern wrote:

since the cat code currently in trunk works as expected.

Actually, the problem was likely that mercurial wasn't getting passed a revision when calling cat, which is also fixed in my upcoming patchset

#15 Updated by Luke Hoersten about 12 years ago

Peter Fern wrote:

Peter Fern wrote:

since the cat code currently in trunk works as expected.

Actually, the problem was likely that mercurial wasn't getting passed a revision when calling cat, which is also fixed in my upcoming patchset

Sounds very likely. I'll give it a try when you're ready.
Thanks for all the work!

#16 Updated by Peter Fern about 12 years ago

I can't set issue relationships, but the patch is at #4455

#17 Updated by Toshi MARUYAMA almost 12 years ago

#18 Updated by Toshi MARUYAMA about 11 years ago

  • Category set to SCM
  • Assignee changed from Jean-Philippe Lang to Toshi MARUYAMA
  • % Done changed from 90 to 0

Note 17 is obsolete. Please see #4455.

#19 Updated by Toshi MARUYAMA almost 11 years ago

  • Status changed from New to Closed
  • Target version set to 1.2.0
  • Resolution set to Fixed

Fixed in r4864.

Also available in: Atom PDF