Bug 3264 - Commandserver: New bookmark is not marked as active
Summary: Commandserver: New bookmark is not marked as active
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: unspecified
Hardware: All All
: normal bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-12 07:35 UTC by Jan Sorensen
Modified: 2012-05-13 05:11 UTC (History)
5 users (show)

See Also:
Python Version: ---


Attachments
(34 bytes, application/octet-stream)
2012-02-12 08:19 UTC, Jan Sorensen
Details
(34 bytes, text/x-python-script)
2012-02-12 08:19 UTC, Jan Sorensen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Sorensen 2012-02-12 07:35 UTC
If you create a new bookmark, and then make a commit, the newly created bookmark is still referencing the old changeset. It should have 
been moved to reference the new commit. It can be reproduced with the following test:

diff --git a/tests/test-commandserver.py b/tests/test-commandserver.py
--- a/tests/test-commandserver.py
+++ b/tests/test-commandserver.py
@@ -179,6 +179,16 @@
     os.system('hg upd bm1 -q')
     runcommand(server, ['bookmarks'])
 
+def bookmarks2(server):
+    # Create new active bookmark 'bm3'
+    runcommand(server, ['bookmarks', 'bm3'])
+    f = open('a', 'ab')
+    f.write('2')
+    f.close()
+    runcommand(server, ['commit', '-Amm'])
+    # bookmarks list bm1 as active, and bm3 still points to the previons changeset. This is bug
+    runcommand(server, ['bookmarks'])
+
 def tagscache(server):
     readchannel(server)
     runcommand(server, ['id', '-t', '-r', '0'])
@@ -208,5 +218,6 @@
     check(hookoutput)
     check(outsidechanges)
     check(bookmarks)
+    check(bookmarks2)
     check(tagscache)
     check(setphase)
Comment 1 Idan Kamara 2012-02-12 07:52 UTC
I'm not seeing the same here. Can you attach a full patch with the matching 
.out file such that it fails when you run it? Also, we're talking about 2.1, 
right?

Btw, you're missing a call to readchannel(server) in the beginning of the 
test.
Comment 2 Jan Sorensen 2012-02-12 08:19 UTC
Yes, this is with 2.1.

Attached the the output as I would expect.
Comment 3 Jan Sorensen 2012-02-12 08:19 UTC
and here is the complete test code
Comment 4 Idan Kamara 2012-02-15 08:01 UTC
OK, after a long and grueling debugging session, I think I know what's 
happening.

We keep a map called _filecache in localrepo. This map, among others 
contains the contents of .hg/bookmarks.current, **as a string**.

So suppose its current contents are 'foo'. Now you run 'hg book bar' through 
the command server. Then the current bookmarks changes: 
localrepo._bookmarkcurrent = 'bar' and .hg/bookmarks.current is written.

Now we run commit: we call localrepo.invalidate, so the next call to 
localrepo._bookmarkcurrent checks if _filecache has the most up-to-date 
contents of .hg/bookmarks.current. It does, so it pulls its OLD contents 
from the cache without rereading the file, and this is where it breaks. It 
doesn't see the new value 'bar' that was set before. The assignment to 
_bookmarkcurrent created a new string and it leaves the old copy in it 
cache.

So basically _filecache breaks when you try to update something in it by 
creating a brand new object.

Incidentally, I found another bug when tracking this one down.
Comment 5 Matt Mackall 2012-02-16 17:07 UTC
Fixed by http://selenic.com/repo/hg/rev/236bb604dc39
Idan Kamara <idankk86@gmail.com>
scmutil: update cached copy when filecached attribute is assigned (issue3263)

(please test the fix)
Comment 6 Jan Sorensen 2012-02-16 17:40 UTC
I have tested it and it is working. Thank you.
Comment 7 Bugzilla 2012-05-12 09:28 UTC

--- Bug imported by bugzilla@serpentine.com 2012-05-12 09:28 EDT  ---

This bug was previously known as _bug_ 3263 at http://mercurial.selenic.com/bts/issue3263
Imported an attachment (id=1630)
Imported an attachment (id=1631)