Bug 40678

Comment 1 Michael Valenta 2005-03-09 12:40:23 EST

*** Bug 87517 has been marked as a duplicate of this bug. ***

Comment 2 Marco Qualizza 2005-03-15 16:25:57 EST

The fix for the immediate problem is actually really easy -- in
RestoreFromRepositoryAction.java, comment out the check for Attic
(AtticLogListener#messageLine) and change 'start' to be
lastIndexOf(Session.SERVER_SEPARATOR).

I encountered NPE's when the repository file doesn't have a corresponding folder
locally (which is a problem anyways, as far as I can tell from the code -- I
don't have the time to actually test the scenario) so I just added a null-check
on currentFolder around the atticFiles.add(...) line (same method).

Comment 3 Michael Valenta 2005-03-15 16:34:31 EST

That is not really enough to fix the problem as it will then show all files 
and not just those that need to be restored (but it is a good start). One 
would then need to remove any resources that exist locally from the list of 
all files. should be easy enough to do but I don't have time to do it myself.

The solution for the missing folder case is to create the missing folder 
locally before restoring the file.

Comment 4 Marco Qualizza 2005-03-15 17:21:05 EST

I stared at it for a really long time before making my changes, and I couldn't
figure out how it actually filters already-existing files.  I *believe* that it
does (I can't be sure because I no longer have root access to our CVS server),
but I can't see the mechanism.

The missing folder case doesn't come up when restoring the file, but rather when
getting the initial list of files (~RestoreFromRepositoryAction.java Line 79). 
What would be the effect of creating a ICVSFolder on a non-existant local
directory?  (Yes, I'm fairly confused, but managing to muddle through some of it
:-) )

Comment 5 Michael Valenta 2005-03-16 09:14:35 EST

The log command used simply fecthes all the file names so I don't think you 
would get the filtering unless you add it explicitly. I used Attic as the 
inclusion filter but it would be just as easy to delegate the request to the 
file that was created. This is a simple fix so I've made it and committed it 
to HEAD.

As for the NPE, it would occur if the current folder could not be determined 
for some reason. I can't see how it would be linked to existance (i.e. the 
ICVSFolder handles the non-existance case). It would more likely happen if the 
format of a message received from the server was not understood. Anyway, 
without a reproducable test case, I can't really say why it is occuring. I've 
added the null check so we won't get the NPE.

Comment 6 Michael Valenta 2005-03-16 09:15:38 EST

Oh, by the way, thanks for your help. It was your initial digging that made 
the eventual solution appearant.

Comment 7 Marco Qualizza 2005-03-16 09:42:52 EST

Thank you for the fix.  I'm still confused about the original code (ie/ how was
it exluding files in Attic which weren't dead? -- Unless cvs log displays them
as being in Attic only after they've been removed?).  I've updated with your
code and removed the null check in order to figure out where the NPE was
happening, and it's easily reproducible if you select a folder which has a file
which hasn't been checked in (and isn't .cvsignore'd).  In that case, it seems
that currentFolder is null.  If you decide to go after this, I'm working in a
branch and not HEAD.  Again, thank you for the fix. :-)

Comment 8 Michael Valenta 2005-03-16 09:54:26 EST

OK, I've reproduced it too. It turns out that adding the null check works fine 
in this case since the offending line is ignored. The offending line is "? 
newfile" which is indicating that there is a new file in the current folder. 
This is not something that needs to be restored so it can be safely ignored.