Item945: RcsLite corrupts file's history
Priority: Urgent
Current State: Closed
Released In: 1.0.1
Target Release: patch
Applies To: Engine
Component: RcsLite
Branches:
Steps to reproduce:
- Adjust
$Foswiki::cfg{StoreImpl}
to RcsWrap
- Create a topic and make some changes forcing new revisions
- Check history. It works.
- Change
$Foswiki::cfg{StoreImpl}
to RcsLite
- Make at least one more revision to the topic
- Check history. It works. (but the history file is already corrupted)
- Change
$Foswiki::cfg{StoreImpl}
back to RcsWrap
- Check history. BOOM
If you try to run
rlog
at command line, you get a error like:
$ rlog data/Sandbox/TestTopic0.txt
RCS file: data/Sandbox/TestTopic0.txt,v
Working file: data/Sandbox/TestTopic0.txt
head: 1.4
branch:
locks: strict
access list:
symbolic names:
keyword substitution: o
total revisions: 4; selected revisions: 4
description:
none
rlog: data/Sandbox/TestTopic0.txt,v: bad diff output line:
@
rlog aborted
RcsLite
adds a blank line before
@
character (
@
marks the end of a revision data).
Thank you very much, Juraj, for giving this feedback!
(Refactored to make
SMART. Check
revision 11 to see all previous trace.)
--
GilmarSantosJr - 07 Feb 2009
I added a unit test (test_Item945 in
RcsTests.pm
) that checks compatibility of histories between
RcsWrap and
RcsLite, and it works fine. Without knowing exactly what changes you made to cause this failure there's not much more I can do. Gilmar, please take a look at that unit test and see if you can make it fail.
--
CrawfordCurrie - 08 Feb 2009
added Michael as he's indicated he had a similar issue.
Oh, and me too - I saw this last week! hopefully one of us will find time.
--
SvenDowideit - 08 Feb 2009
numRevisions
works cause it calls
rlog -h
, that doesn't parse the entire file. I modified the test, so the created revisions are the same (2 from
RcsWrap
and 2 from
RcsLite
) and, after each new created revision, check history and revision info (to force the file to be parsed).
--
GilmarSantosJr - 08 Feb 2009
I have been using rcs lite for too long. As a consequence I plain had to throw away
ALL
of my ,v files as they were corrupted in ways I was not able to figure out how to repair them
That' said we might need a way to
repair
broken ,v files as people might get into the same situation as I was.
--
MichaelDaum - 08 Feb 2009
Don't throw away your
,v
files, Michael! The corruption exposed by this task is an extra
\n
in some fields. I think we should provide a repair script along the fix to this issue (if we confirm the problem is always an extra
\n
).
--
GilmarSantosJr - 08 Feb 2009
I agree
fix-it code should be possible, and will be desired - I think I'll be needing it in the next month or 2
good job on the unit tests
--
SvenDowideit - 08 Feb 2009
I found two issues with the code:
- The stored
log
message always terminate with one, and only one, \n
(even if the original message has multiple or none \n
. This is the way rcs works. Verify at command line).
- The
\n
is not part of the log message
- The stored log message should match
/@((\n?[^\n])+\n)?@/
- rcs always stores log messages according to this regex, but it doesn't complain if it finds a message that doesn't match
-
RcsLite
wrongly added an extra empty line for each transformation
The history file stores the latest revision unmodified and the previous ones as reverse diffs: apply the diff stored at revision
head - 1
to
head
and you'll get
head - 1
text. Apply diff stored at revision
head - 2
to
text of revision
head - 1
and you'll get
head - 2
text and so on. To add a revision, the old head text is compared to the new text and replaced by the diff. This is what I called
transformation.
The fix script should:
- Canonicalize log messages
- Remove the extra empty line (if it exists) from revision diffs
- We know that an empty line is extra based on the lines counter of the operation.
For now I improved the tests and fixed the issue. No time to work on the script. Feel free to develop it
--
GilmarSantosJr - 09 Feb 2009
In SVN-Change 2424 at the end of the
_addChunk
method the line
$out .= "\n";
was removed, but now there is a problem: RcsLite creates corrupt files (whenever a "d"-change is the last change in a revision): Example
text
@d1 1
a1 1
d3 1@
Now the
newline is missing after "d3 1"; at least
rlog
says:
rlog: ./RcsTest.txt,v: bad diff output line: d3 1@
rlog aborted
I think in this method for "d"-changes we have to use:
$$out .= 'd' . ( $start + 1 ) . ' ' . $nLines."\n";
Because in my opinion after
every "d"-change a newline is needed.
--
ChristianLudwig - 19 Feb 2009
Oh, thank you
ChristianLudwig. I was starting to get crazy. I tried this on Lenny and
OpenSolaris, and it failed.
EugenMayer tried it on Intrepid, and it worked. I guess he just didn't make a d change, and this worked out.
I'll try to fix it properly, and also to fix the unit test too.
--
OlivierRaginel - 20 Feb 2009
We're still missing the migration script, but in fact, we might not need it. If you simply use RcsLite to make a new revision on your topic(s), it should rewrite it properly. And it should be able to read the buggy topics too.
So the script should simply be a loop on the topics, using the latest RcsLite, and writing it back.
I could write it, but I'd rather first wait for a few of the other players involved to confirm this works.
Patch resides in
distro:ca794672ac6a (did a few other commits to split things up).
--
OlivierRaginel - 21 Feb 2009
This all sounds scarily familiar; I had previously (back in TWiki 4.1 time) fixed several other similar errors. There really needs to be a more thorough unit test, that covers all the delta operators that RcsLite (and RCS itself) can generate.
--
CrawfordCurrie - 23 Feb 2009
Which was exactly why I re-wrote the unit test the way I did it.
Here, you can easily expand it so that it generates whatever you want, with whatever RCS implementation you want, by adding lines to the @historyItem945 array in
UnitTestContrib/test/unit/RcsTests.pm
As I have no clue what they are, or how they are triggered, I only did what I could.
- First parameter is the implementation (remember, the tests will be run also by each implementation, and the resulted ,v will be compared too)
- Second parameter is the entire text countained in the topic, thus the diff will be made with the previous line
- Third and Fourth parameters are the user and comment
- Sixth parameter is the time
In brief, they will be passed as-is to the addRevisionFromText method of the implementation chosen as first parameter.
--
OlivierRaginel - 23 Feb 2009
I am closing this bug report and adding it to the 1.0.1 release fixes.
If there are additional tasks after 1.0.2 open a new report.
I do this to have a closed report in the release note. It is an urgent important bug to report fixed.
--
KennethLavrsen - 24 Feb 2009
Added repair script; tools/repair_rcs_lite.pl; see
Item3212
--
CrawfordCurrie - 16 Jun 2009