On 18.03.2018 13:43, Julian Foad wrote:
> Branko Čibej wrote:
>> On 16.03.2018 18:33, Julian Foad wrote:
>>> I implemented an output format compatible with 'svn-viewspec.py' in
>>> r1826990. Then I updated that output format to also support 'switched'
>>> and revisions, in r1826993. This version outputs a header declaring
>>> 'Format: 2', and svn-viewspec.py currently barfs on reading that.
>>>
>>> Suggested exercises for the reader (you, plural):
>>>
>>> * implement a UI to choose the output format (currently 'svn info
>>> --viewspec' is hard-coded to produce that 'format 2')
>>
>> Why?
>
> To experiment with the two existing implementations of "recreate WC
> from a viewspec" -- which are direct command-line cmds and
> svn-viewspec.py.
Again, why? Pick one and use it. It's not as if there was some subtle
tradeoff here that needs lots of experimenting with two options in order
to reach a decision. Also: svn-viewspec.py uses command-line cmds so
there's no difference in practice (which leads to my other discussion,
see below).
>> 'svn-viewspec.py' is not a supported tool; 'svn info --viewspec'
>> does not exist in 1.10. We don't have any compatibility guarantees to
>> worry about.
>
> Yes.
>
>>> * update 'svn-viewspec.py' to implement those 'format 2' extensions
>>>
>>> * start implementing the API functions that 'svn-viewspec.py' needs
>>
>> Again, why? 'svn-viewspec.py' uses the standard command-line tools. It
>> follows that all API functions it needs already exist.
>
> An efficient implementation is not yet possible. New APIs are needed.
> The existing implementation checks out the root, then modifies each
> subdirectory in depth-first order. If the root is depth-infinity, then
> it first checks out everything, then proceeds to delete the unwanted
> bits (and the same applies at any level where depth is reduced).
That is an inefficiency in the implementation of svn-viewspec.py, not a
lack of APIs.
> We need to design and implement the APIs that we will need to make an
> efficient implementation.
I disagree. I've seen no overwhelming argument in favour of requiring
new APIs for this. Your argument is "efficiency" but in terms of what?
The number of client operations? Network data transfer? Or perhaps
efficiency of long-term support for new APIs?
If the issue is data transfer (and local I/O), then the onus is on
svn-viewspec.py to properly analyse the viewspec and construct a minimal
set of client operations to reproduce the working copy structure. If you
invent new APIs to do that instead, you'll have to support them
indefinitely in the client library, and that is, IMO, a less efficient
use of available resources.
> Working with bindings to APIs seems like a good way to do that.
Sure, but not at this point in time. We should only add new APIs as the
existing client(s) need them. On the other hand, svn-viewspec.py is a
good vehicle for developing the necessary methods for efficiently
reproducing working copy topology.
I suggest things should be done in this order:
1. Fix the most glaring problems in svn-viewspec.py (such as use of
system() instead of popen)
2. Improve the algorithm for constructing a working copy from the
viewspec, using only existing command-line client operations
3. Consider what kind of client operations (implying new or improved
APIs) are required to optimise the algorithm further, and implement
those (note: exposing these operations in the command-line client
will give users access to new functionality without having to resort
to using the APIs directly)
4. Once the above is done, we can then decide to either convert
svn-viewspec.py to use the bindings, or to implement 'svn checkout
--viewspec' instead
"Adding new APIs" seems like an obvious way to proceed, but we have
long-standing counter-examples in our code:
* the MTCC API, which was experimental and is now private and used by
exactly one tool, and which, as far as I can remember, no-one has
really requested was made public;
* editor v2 ... unfortunately (by my error in judgement) exposed in
JavaHL but, again, not as a public C API.
-- Brane
Received on 2018-03-18 14:12:56 CET