Item10427: rev details lost if .txt file is edited externally

Lavr noted that if a topic txt file has been externally edited, Foswiki uses the system file timestamp to determine if a change is within the horizon to not create a new revision. This behavior has been there since the (tm)wiki days.

A simple fix would be to base the comparison on the timestamp of the rcs file instead of the topic file. As long as the rcs file was not manually edited as well, the last revision time will likely be older and the first save after external hacking will be more likely to create a new revision instead of merging the Foswiki (and external) changes into the last rev.

diff --git a/core/lib/Foswiki/Store/VC/Handler.pm b/core/lib/Foswiki/Store/VC/Handler.pm
index c09ff03..1ed5af4 100644
--- a/core/lib/Foswiki/Store/VC/Handler.pm
+++ b/core/lib/Foswiki/Store/VC/Handler.pm
@@ -318,7 +318,9 @@ Get the time of the most recent revision
 =cut
 
 sub getLatestRevisionTime {
-    my @e = stat( shift->{file} );
+    my $this = shift;
+    my $file = ( -e $this->{rcsFile} ) ? $this->{rcsFile} : $this->{file};
+    my @e = stat( $file );
     return $e[9] || 0;
 }

-- GeorgeClark - 01 Mar 2011

Can of worms. I think we need to break the problem down a bit, so we can target the discussion more accurately.

Firstly, let's be sure that we understand that this problem is specific to the VC store, and is not fundamental to Foswiki. Foswiki requirements on the store are quite simple, in that it expects that all changes to a topic are recorded in the revision history unless a change occurs within a certain time of a prior change, in which case the changes will be merged into the previous revision. The store is expected to:

• provide the means to create a new revision, given new content (save)
• provide a way to merge new content into the HEAD revision (repRev)
• provide a way to delete the latest revision (or promote the previous revision back to the HEAD)

and that's all. The unit tests we have are principally designed to ensure these requirements are met.

Now, we also have some other external, non-foswiki, behaviours of the VC store (note; behaviours, not requirements):

1. The latest revision of a topic is available as a text file
2. That text file can be modified externally
3. If that file has been modified by something other than Foswiki, then
   1. those changes will be seen in Foswiki as part of the most recent revision
   2. the changes will be incorporated in any Foswiki save as part of the new revision

Those behaviours can be seen as de facto requirements, because "that's the way it's always worked". OK, fine, but we have to clearly separate the two sets of requirements in the unit tests, both to help us express the second set more clearly, and also to ensure those requirements don't pollute other store implementations. Some other things are rather unclear, and also need to be disambiguated:

1. If I have a revision X, and the text file is modified, should Foswiki see that as revision X, or revision X+1?
   • If the changes are saved in Foswiki as described in behaviour 3.2, they become part of X+1, not X
2. If the text file is externally modified (creating implicit X+1), and delRev is executed in Foswiki, which rev is restored? X-1, X, or the implicit X+1?
3. If an attachment is added to a topic with implicit X+1, should the attachment be added to X+1, or should X+1 be committed and X+2 get the attachment?
4. If the text file (which has TOPICINFO{version=X}) is externally modified, but the TOPICINFO{version} is not incremented, and that file is checked in to RCS (again by an external agency, not by Foswiki), how does Foswiki resolve the fact that there are now two revisions with the same TOPICINFO{version=X} in the history, but no TOPICINFO{version=X+1}?

All these cases (and more besides) need to be covered by VC-specific unit tests.

-- CrawfordCurrie - 01 Mar 2011

You are making it more complicated than it needs to be.

All we need to do is make the repRev decision based on the %META:TOPICINFO timestamp if it is there. Or the rcs file timestamp.

Basing it on the timestamp of the file is wrong. It means that if you hack the file and someone edits and save the topic inside Foswiki within the repRev interval then a version is lost. This is bad.

If there is no ,v file present there is no repRev to consider. So if you edit and save a topic without a ,v file it is NEVER a repRev situation. Then it is a new topic and a rev 2 gets created and a ,v file created with the original as rev 1.

It is not such a huge can of worms.

-- KennethLavrsen - 06 Mar 2011

I'm not making it any more complicated than it is. You are thinking in the right direction, but you need to consider all the cases, so that the unit tests cover the edge cases and we don't end up banjaxing something. We arrived at the current state because no-one has bothered to do this before, but has tried to patch point solutions to perceived problems.

If we follow your logic, then the answers to my questions are:

1. If I have a revision X, and the text file is modified, should Foswiki see that as revision X, or revision X+1?
   • If the META:TOPICINFO is there in the cache, and we assume it has not been manually modified in the edit, then the manually modified cache version is seen as the current version. If Foswiki is saving new text for the topic (not based on the existing rev) then the manual changes will be lost.
   • If there is no META:TOPICINFO then the timestamp of the ,v is taken (putting aside questions f whether the ,v or the cache is modified last in a standard save) then Foswiki can "see" that the cache is more recent and can create an X+1 on the fly. This works as long as the cache is always older than (or the same age as) the ,v
2. If the text file is externally modified (creating implicit X+1), and delRev is executed in Foswiki, which rev is restored? X-1, X, or the implicit X+1?
   • If a delRev is treated as a "check the status and then delete" then the same rules (and same problems) apply.
3. If an attachment is added to a topic with implicit X+1, should the attachment be added to X+1, or should X+1 be committed and X+2 get the attachment?
   • Ditto
4. If the text file (which has TOPICINFO{version=X}) is externally modified, but the TOPICINFO{version} is not incremented, and that file is checked in to RCS (again by an external agency, not by Foswiki), how does Foswiki resolve the fact that there are now two revisions with the same TOPICINFO{version=X} in the history, but no TOPICINFO{version=X+1}?
   • See 1

If we are able to describe all the unit tests, then we can use whatever implementation satisfies those tests.

BTW note that the META:TOPICINFO cannot be trusted if (1) no ,v file exists or (2) the cache file has been manually modified. There are four possible cases for a manual edit:

1. TOPICINFO was not present before the edit started, but has been added during the manual edit
2. TOPICINFO was not present before the manual edit started, and is still not present
3. TOPICINFO was present, but has been changed by the manual edit
4. TOPICINFO was present, and is still unchanged from since before the manual edit

AFAIK the only way to test if the cache has been modified more recently than the ,v is to use file dates. Unless someone knows better?

-- CrawfordCurrie - 13 Mar 2011

This task is superseded by Item11091

-- CrawfordCurrie - 31 Aug 2011

Commenting is disabled while not logged in