Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 320686 - Cannot compare locked revision with another regular revision
Summary: Cannot compare locked revision with another regular revision
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: CVS (show other bugs)
Version: 3.6   Edit
Hardware: PC All
: P3 major (vote)
Target Milestone: 3.7 M2   Edit
Assignee: Tomasz Zarna CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-22 22:28 EDT by Olexiy Buyanskyy CLA
Modified: 2010-08-17 09:32 EDT (History)
2 users (show)

See Also:


Attachments
Patch to fix setRevision function (1.49 KB, patch)
2010-07-22 22:28 EDT, Olexiy Buyanskyy CLA
no flags Details | Diff
example of error (8.28 KB, image/png)
2010-07-26 14:52 EDT, Olexiy Buyanskyy CLA
no flags Details
cvs log output (1.28 KB, text/plain)
2010-07-27 04:51 EDT, Tomasz Zarna CLA
no flags Details
reproduced issue with cvsnt 2.5.04 (14.25 KB, image/png)
2010-07-27 11:40 EDT, Olexiy Buyanskyy CLA
no flags Details
patch to fix setRevision function and parsing of log information (5.03 KB, patch)
2010-07-28 16:47 EDT, Olexiy Buyanskyy CLA
no flags Details | Diff
Patch v03 (4.89 KB, patch)
2010-08-13 05:10 EDT, Tomasz Zarna CLA
no flags Details | Diff
mylyn/context/zip (93.71 KB, application/octet-stream)
2010-08-13 05:10 EDT, Tomasz Zarna CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Olexiy Buyanskyy CLA 2010-07-22 22:28:01 EDT
Here is use case. We use locking capabilities of CVS with command

cvs admin -lbranch file

Open CVS History of your locked file.
Select revision that is locked and pick another revision. You will see an error in compare view for locked one.
You can even lock both revisions and will get an error for both sides :-)
This bug somehow related to bug 103374 and the one I have used to attach my patch bug 44438.
This functionality is critical for our company. I hope to get this patch applied for eclipse 3.6.1.
Comment 1 Olexiy Buyanskyy CLA 2010-07-22 22:28:33 EDT
Created attachment 175037 [details]
Patch to fix setRevision function
Comment 2 Tomasz Zarna CLA 2010-07-26 09:39:23 EDT
I locked all files using "cvs admin -L", opened History view and picked two revisions (the latest was locked) and it worked for me. I'm sure I'm missing something. Am I able to reproduce the steps you referring to in comment 0 using CVS Watch/Edit options?
Comment 3 Olexiy Buyanskyy CLA 2010-07-26 09:44:14 EDT
cvs admin -L does not lock. You have to use cvs admin -l (lower case)
Comment 4 Tomasz Zarna CLA 2010-07-26 10:01:01 EDT
I did. This is output from my console:

cvs -d :pserver:cvsadmin@localhost:/BUGS admin -l file1.txt 
RCS file: /BUGS/bug320585_compareLocked/file1.txt,v
1.2 locked
done

Then, in the previously checked out project, I opened History view for file1.txt, selected two revs (1.1 and 1.2 - locked) and did "Compare with Each Other". Worked fine. Am I doing it in the right order?
Comment 5 Olexiy Buyanskyy CLA 2010-07-26 14:44:58 EDT
Can you give cvs log of the file? Maybe that is specific to cvs server we use 1.11.23 version
Comment 6 Olexiy Buyanskyy CLA 2010-07-26 14:50:51 EDT
Here is my example of log after locking

revision 1.1    locked by: obuyansk;
date: 2010/07/14 15:02:41;  author: obuyansk;  state: dead;
branches:  1.1.2;
file sellingToolRC.h was initially added on branch br_REL-070710.
----------------------------
revision 1.1.2.2        locked by: obuyansk;
date: 2010/07/25 20:49:15;  author: obuyansk;  state: Exp;  lines: +2 -1
Use more generic macro in case of not found project ST_CGI_NOT_FOUND
----------------------------
revision 1.1.2.1        locked by: obuyansk;
date: 2010/07/14 15:02:41;  author: obuyansk;  state: Exp;  lines: +118 -0
Comment 7 Olexiy Buyanskyy CLA 2010-07-26 14:52:38 EDT
Created attachment 175259 [details]
example of error

This example show how error looks like
Comment 8 Tomasz Zarna CLA 2010-07-27 04:51:04 EDT
Created attachment 175296 [details]
cvs log output

I'm running local CVSNT 2.5.03 (Scorpio) Build 2704.
Comment 9 Olexiy Buyanskyy CLA 2010-07-27 11:38:55 EDT
I was able to reproduce this with 
Server: Concurrent Versions System (CVSNT) 2.5.04 (Zen) Build 3236 () (client/server)

$ cvs admin -l1.2 flexelint_lib.mk
RCS file: /cvsroot/tools/com.lowes.eclipse/com.lowes.eclipse.cdt/qibmake/share/flexelint/flexelint_lib.mk,v
1.2 locked
done

See next screen short of error. I was able to reproduce same error.
Comment 10 Olexiy Buyanskyy CLA 2010-07-27 11:40:43 EDT
Created attachment 175329 [details]
reproduced issue with cvsnt 2.5.04
Comment 11 Olexiy Buyanskyy CLA 2010-07-28 11:02:43 EDT
Tomasz,

I also see bug issue in history view as well. It does not show list of tags for locked revision.

that is because in /org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/client/listeners/LogListener.java

we does not parse properly revision number

    			} else if (line.startsWith("revision ")) { //$NON-NLS-1$
    				revision = internAndCopyString(line.substring(9));
    				state = REVISION;

I am thinking to create function to remove LockedBy message and call from both places.
Can I use /org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/util/Util.java ?
Comment 12 Olexiy Buyanskyy CLA 2010-07-28 16:47:32 EDT
Created attachment 175444 [details]
patch to fix setRevision function and parsing of log information

this patch include previous one
Comment 13 Tomasz Zarna CLA 2010-08-03 06:24:23 EDT
All righty, I finally managed to reproduce *both* of the cases (comment 0 and comment 11). I'll review your patch in the first week of 3.7M2.

(In reply to comment #0)
> I hope to get this patch applied for eclipse 3.6.1.

I don't think this is critical enough for 3.6.1, the problem has been there for a while so it can't be that critical.

Actually, I don't even think this bug is critical at all. You're not experiencing any "crashes, loss of data, severe memory leak"[1] so I'm lowering the severity to Major.

[1] https://bugs.eclipse.org/bugs/page.cgi?id=fields.html#bug_severity
Comment 14 Olexiy Buyanskyy CLA 2010-08-03 10:30:33 EDT
Thanks for quick reply. I really appreciate this. 
I just hope that during review of patch you will find it how simple it's.
it does not brake API and should be easy to apply for 3.6 and even 3.5.

We currently use 3.5 and we plan to use 3.6 in couple of months and we have more then 100 of developers using eclipse.
I always have a choice to manually patch eclipse as I did for 3.5, I just hoped that I will not need to do that for 3.6.
Comment 15 Andrew Gvozdev CLA 2010-08-03 10:36:00 EDT
For those who use CVS locks the issue is pretty severe as the locks could stay for days. You cannot do any comparison with the most interesting version and some developers tend to wait out until the lock is dropped.
Also, since you can't see the tags you can't tell for sure which revision went to the release.

> I don't think this is critical enough for 3.6.1, the problem has been there for a while so it can't be that critical.
Is it possible to reconsider in favor of adding to 3.6 maintenance release? AFAICS it is a clear bug, the patch is small and all the changes are internal.
Comment 16 Tomasz Zarna CLA 2010-08-13 05:10:19 EDT
Created attachment 176528 [details]
Patch v03

A simpler version of Olexiy's latest patch. I would like to release this one to HEAD, is that ok with you?
Comment 17 Tomasz Zarna CLA 2010-08-13 05:10:35 EDT
Created attachment 176529 [details]
mylyn/context/zip
Comment 18 Olexiy Buyanskyy CLA 2010-08-13 09:02:43 EDT
looks perfect. I like idea with regular expression string.
Comment 19 Tomasz Zarna CLA 2010-08-16 10:44:13 EDT
(In reply to comment #15)
> AFAICS it is a clear bug, the patch is small and all the changes are internal.

This is all true, but it is not enough. We usually backport only severe issues which lead to crashes, data loss or at least major loss of function. If the bug is a regression it's also more probable it will be backported. If we hadn't kept to these strict rules we would have ended up in backporting all major bugs, which doesn't make much sense to me.
Comment 20 Tomasz Zarna CLA 2010-08-16 11:01:43 EDT
The latest patch has been applied to HEAD. It will be available in builds >N20100815-2000. Olexiy, thanks for the initial patch. Please verify the fix, when you find some spare time.
Comment 21 Olexiy Buyanskyy CLA 2010-08-17 09:02:22 EDT
(In reply to comment #20)
> The latest patch has been applied to HEAD. It will be available in builds
> >N20100815-2000. Olexiy, thanks for the initial patch. Please verify the fix,
> when you find some spare time.

Verified. All functionality works in N20100815-2000.
Comment 22 Tomasz Zarna CLA 2010-08-17 09:32:56 EDT
Thanks. Marking as VERIFIED.