Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 353875 - Replace with Revision fails to notice trivial changes on locked files
Summary: Replace with Revision fails to notice trivial changes on locked files
Status: RESOLVED FIXED
Alias: None
Product: Subversive
Classification: Technology
Component: Core (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Igor Burilo CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-04 07:59 EDT by Neels Hofmeyr CLA
Modified: 2012-05-15 11:28 EDT (History)
1 user (show)

See Also:


Attachments
discard trivial error by comparing file (first size, and, if needed, then content as well) (1.93 KB, patch)
2011-08-04 08:17 EDT, Neels Hofmeyr CLA
i.vinnykov: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Neels Hofmeyr CLA 2011-08-04 07:59:19 EDT
Build Identifier: 20110615-0604

If a file is locked (i.e. svn:needs-lock is set, so the WC file is read-only in the locked state), and if this file is part of a replace-with-revision operation, and if furthermore the current file content is *identical* to the content that should replace it, an Exception interrupts the operation.

This is a trivial situation: For example, a user chooses to roll back to, say, HEAD minus 1. Say only two or three (unlocked) files have changed, but here and there are a few binary files, with svn:needs-lock set, scattered around the working copy. The revision that should be rolled back has no modifications to any of those locked files, so they could simply be skipped altogether. If the user chooses 'Replace with revision', this is what currently happens:

- Subversive downloads the same binary file(s) from the repos (export to temp)
- Subversive tries to copy the downloaded file over the current WC file
- the current file is locked and read-only, the copy fails on fs level (Permission Denied).

This failure is completely unnecessary, as the locked files are not supposed to change at all anyway. If it weren't for the lock, they would merely be overwritten with identical content.

(If the files were different, the user should have to lock the files first and then do the replace with revision. But for non-changing files, this is annoying.)

There is a workaround, which is to use a single-URL reverse merge instead. But this is more complex to figure out for non-Subversion-proficient users. Ideally, the replace-with-revision should actually invoke a reverse merge, as this would also correctly treat all file properties and be more optimal, as unchanged file content is not even downloaded from the server.

But changing this behavior might have unwanted side effects and is harder to code. Instead, I've found a "dirty hack" kind of fix that discards exceptions that happen on files that are anyway identical (will attach).

Note that this is related to bug #294610, but this bug report is about *unmodified* locked files, while #294610 talks about any locked files in general.
( https://bugs.eclipse.org/bugs/show_bug.cgi?id=294610 )


Reproducible: Always

Steps to Reproduce:
1. First, create a simple testing repos. Here is a unix shell script:

svnadmin create /tmp/repos
svn co file:///tmp/repos /tmp/wc
cd /tmp/wc

# create a trivial text file
echo x > textfile
svn add textfile

# create a trivial binary file (simply copy the basename binary)
# and set it to needs-lock.
cp `which basename` binfile
svn add binfile
svn ps svn:needs-lock y binfile

svn ci -m"Files created"

# add a mod on the text file
echo y > textfile
svn ci -m"textfile mod"

2. Checkout the entire repos in Subversive.

3. Right-click on the working copy root ("repos") and choose
   Replace with --> Revision or URL

4. Choose Revision, enter number 1. 

5. Click OK and say yes, you really want to replace.

6. An error occurs complaining about Permission Denied on binfile. binfile is not supposed to change at all, there is only this one revision of it in the repos.
Comment 1 Neels Hofmeyr CLA 2011-08-04 08:17:26 EDT
Created attachment 200910 [details]
discard trivial error by comparing file (first size, and, if needed, then content as well)
Comment 2 Alexander Gurov CLA 2011-10-02 04:20:28 EDT
Hi Neels,

I've applied the patch with some insignificant changes. So, please check if there are any changes in the logic of what it is doing now (I'm pretty sure there shouldn't be any).
Comment 3 Neels Hofmeyr CLA 2011-10-06 10:52:45 EDT
Thanks for applying the patch! (r21043)

I see that you have changed the order, so the code checks beforehand if the destination file is not writable. It also properly closes the BufferedReader/Writer. I think your tweaks make it less fragile, very good.
Comment 4 Igor Vinnykov CLA 2012-05-15 11:28:25 EDT
Comment on attachment 200910 [details]
discard trivial error by comparing file (first size, and, if needed, then content as well)

Set iplog flag