RE: [PATCH v2] Reject checkouts to existing directory

Just checking the log message: This changes 90% of our testsuite to use ‘svn co --force’, by changing the standard checkout function in actions.py that is used by virtual all tests.

If that is really necessary I would call this a breaking change.

It also changes a few of the most core basic tests… That tells me that some very common usecases will change.

If we have to change the behavior of 90% of our tests for this, I don’t think we can just commit it as any basic minor test.

In the summary I don’t see the support for restartability of a broken checkout. See the documentation of svn checkout in the svn-book. (http://svnbook.red-bean.com/nightly/en/svn.ref.svn.c.checkout.html)
[[
If you interrupt a checkout (or something else interrupts your checkout, such as loss of connectivity, etc.), you can restart it either by issuing the identical checkout command again or by updating the incomplete working copy:
$ svn checkout file:///var/svn/repos/test mine
A mine/a
A mine/b
^C
svn: E200015: Caught signal
$ svn checkout file:///var/svn/repos/test mine
A mine/c
^C
svn: E200015: Caught signal
$ svn update mine
Updating 'mine':
A mine/d
Updated to revision 20.
$
]]
Resuming will now require a –force, which also triggers different behavior in other parts of checkout.

Which would require me to update dozens of buildscripts that worked just fine since 1.0, if released.

            Bert

Sent from Mail for Windows 10

From: Stefan Sperling
Sent: donderdag 27 oktober 2016 21:45
To: Patrick Steinhardt
Cc: Subversion; Stefan Sperling; Ivan Zhakov; Stefan
Subject: Re: [PATCH v2] Reject checkouts to existing directory

On Tue, Oct 18, 2016 at 02:22:25PM +0200, Patrick Steinhardt wrote:
> Hi,
>
> finally got around to update my patch regarding checkouts to
> existing directories. The semantics have been changed to accept
> checkouts iff
>
> - the target directory does not exist
> - the target directory is empty
> - the repository to check out is empty
> - the --force flag is given

I think this makes a lot of sense. I just have not yet had time to
review and test your patch. I will try to do so soon.

Does anyone reading this list have any concerns about this change?
Received on 2016-10-28 02:49:10 CEST