MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0008168CMakeCTestpublic2008-11-25 10:422009-02-25 14:59
Reportersimonh 
Assigned ToBrad King 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake-2-6 
Target VersionFixed in Version 
Summary0008168: CTest retrieval of file versioning history has logic errors
DescriptionSource/CTest/cmCTestUpdateHandler.cxx has various logic errors and inconsistencies.

From CMake-2.6.2:
Line 707: I think the logic is reversed here - modifiedOrConflict should be true if any of these characters are present or indeed 'U'. At least, logging information should be retrieved if "svn update" changes a file - returns "U filename".

Line 726: svn log can cope with "log -r M:N" whether M >= N or N =< M. svn status shouldn't really be needed (perhaps only if svn_latest_revision discovery failed).

Line 758: This boolean is set outside this context (line 445, 493, 560, 581, ...) and by any previous loop evaluation at line 749. The condition should be nested within if(modifiedOrConflict)
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
duplicate of 0008128closed Brad King CMake ctest cannot handle SVN usernames with spaces 
related to 0010484closed Brad King CMake There is no way to run svn/git/hg with custom options like --username. 
child of 0007541new  CDash support modern version-control tools 
Attached Filespatch ctest-versioninfo.patch (27,283) 2008-11-25 10:46
https://public.kitware.com/Bug/file/1867/ctest-versioninfo.patch
Issue History
Date ModifiedUsernameFieldChange
2008-11-25 10:42simonhNew Issue
2008-11-25 10:44simonhNote Added: 0014178
2008-11-25 10:46simonhFile Added: ctest-versioninfo.patch
2008-11-25 10:49simonhNote Added: 0014179
2009-02-12 12:12Bill HoffmanStatusnew => assigned
2009-02-12 12:12Bill HoffmanAssigned To => Brad King
2009-02-13 16:30Brad KingRelationship addedduplicate of 0008128
2009-02-13 16:37Brad KingNote Added: 0014934
2009-02-16 10:03Brad KingNote Added: 0014944
2009-02-25 14:58Brad KingRelationship addedchild of 0007541
2009-02-25 14:59Brad KingNote Added: 0015390
2009-02-25 14:59Brad KingStatusassigned => closed
2009-02-25 14:59Brad KingResolutionopen => fixed
2011-01-17 15:58Brad KingRelationship addedrelated to 0010484

Notes
(0014178)
simonh   
2008-11-25 10:44   
This is related to 0008128 insofar as svn status can be called by condition at line 726 and the output is no always parsed correctly if users can have spaces in names.
(0014179)
simonh   
2008-11-25 10:49   
I've attached a patch which addresses the following:
1) Use "svn status --xml" and parse output using another cmXMLParser derived class
2) modifiedOrConflict is set whenever the "mod" char is M,C,G or U
3) the logcommand is run when modifiedOrConflict is true and only in this branch is the (res) boolean tested and version info logged.
(0014934)
Brad King   
2009-02-13 16:37   
The original author of the update code no longer works with us, so I've been working to understand the code too. As I said in issue 0008128 I've been refactoring this code. I'll work your patch into the refactored version.

I'm pretty sure that modifiedOrConflict is just a misnamed variable. The logic makes sense if the variable were named notModifiedOrConflict...it only gets the svn log for a file if it was not listed by svn due to local modification.
(0014944)
Brad King   
2009-02-16 10:03   
I've committed the spelling and 'res' variable logic fixes. This includes renaming modifiedOrConflict to notLocallyModified for clarity.

STYLE: Fix spelling in cmCTestUpdateHandler
/cvsroot/CMake/CMake/Source/CTest/cmCTestUpdateHandler.cxx,v <-- Source/CTest/cmCTestUpdateHandler.cxx
new revision: 1.52; previous revision: 1.51

BUG: Fix svn update logic for modified files
/cvsroot/CMake/CMake/Source/CTest/cmCTestUpdateHandler.cxx,v <-- Source/CTest/cmCTestUpdateHandler.cxx
new revision: 1.53; previous revision: 1.52

I've rebased the rest of my refactoring on these changes but it isn't yet ready for commit.
(0015390)
Brad King   
2009-02-25 14:59   
I've committed a whole new SVN updates implementation as part of the VCS refactoring in issue 0007541. I also updated the CTest.UpdateSVN test to put a space in the author name.