Re: Consistency between 'update' and 'switch' APIs

C. Michael Pilato wrote:

> On 03/13/2013 01:36 PM, Julian Foad wrote:
>> The 'ignore_ancestry' parameter at the RA level is changing the way diffs
>> of replaced files are reported by the server.  It is not the same as the
>> '--ignore-ancestry' flag that 'svn switch' accepts.  'svn switch
>> --ignore-ancestry' causes libsvn_client to bypass a check that the switch
>> source and destination URLs have a common ancestor before starting the
>> switch.
>>
>> The '--ignore-ancestry' flag also sets this 'ignore_ancestry' parameter
>> in the RA call to cause the server to send diffs of replaced files in a
>> different way, but that is a secondary effect.  It controls whether the
>> diff of a file that happens to exist at the same relative path is
>> reported as text-modify or as delete-and-add, and so affects whether and
>> how conflicts are raised if the user had local mods.
>>
>> It is this 'do diffs as text-mods' meaning that we should make
>> consistently available to both 'update' and 'switch' at the RA level.
>
> Ah, yes, I forgot about the multiple meanings of this flag in the different
> layers.  So I agree, both switch and update should carry the
> server-recognized meaning of 'ignore-ancestry' in both the client and RA
> layers.  (Sorry for the mental lapse.)

PROPOSAL, RECOGNIZING BOTH MEANINGS OF IGNORE-ANCESTRY

  - svn_ra_do_update2():
    Revise to svn_ra_do_update3(), add the 'ignore_ancestry' option there too, and use two pools while we're at it.  Track this change in the RA vtable 'update' method, protocols, etc.

  - svn_wc__get_switch_editor():
    Add 'adds_as_modification'.

  - svn_client__update_internal(), svn_client_update4():
    Add 'ignore_ancestry', which just maps to svn_ra_do_update3(ignore_ancestry), unlike in the corresponding 'switch' APIs where it also allows the source and dest URLs to be unrelated.

  - svn_client__switch_internal(), svn_client_switch4():
    Add 'adds_as_modification'.

> This may be a bit of a sidebar, but:  Does svn_ra_do_status() need the same
> treatment?  I mean, to the degree that 'svn status -u' is supposed to be a
> particularly shaped type of dry-run update, does it also need support for
> the --ignore-ancestry flag?

The interesting case here is a file replaced with an identical file: that's the only case where it's unchanged if ignoring ancestry but otherwise it's changed.  A normal update would try to replace it in the WC, potentially raising a tree conflict if it was locally modified.  An update ignoring ancestry would cause no change for that node.  It is logically sound that we might want 'status' to be able to act in either of these ways, especially if we are using it to decide whether 'update' will make a change.

But what if we are using 'status -u' to decide whether the WC is up to date enough to be able to commit?  In that case, the definition of 'up to date' is fixed by the server side, and treats replacement as a change.  (Unless we contemplate adding 'ignore-ancestry' on commits as well, which I think would only have an effect in that specific situation of a replaced-but-identical file, and that seems waaaay too bizarre to contemplate.)

I am starting to wonder if it wouldn't be a better idea to stop supporting 'ignore ancestry' at the RA layer at all.  What is the compelling use case for this functionality?  And what will we do when we add support for rename tracking, which essentially requires us to track node ids?  You can't then just skip a node if it's identical, because you will still need to report that its id changed.  I'm sure we could make the 'ignore ancestry' concept still work by passing the node-id change out of band, but that's starting to sound really nasty.

If the RA would no longer support 'ignore ancestry', the client side could still coalesce a delete and an add into a diff when it wants to (with some care taken to ensure we don't needlessly re-transmit the whole file content).

Can anyone answer why this ignore-ancestry support was added to 'switch' in the first place (I mean at the file-diff level, not the high-level check for related URLs), or if it just happened by accident?

PROPOSAL, WITHOUT INCREASING THE SUPPORT FOR IGNORE-ANCESTRY

  - svn_wc__get_switch_editor():
    Add 'adds_as_modification'.

  - svn_client__switch_internal(), svn_client_switch4():
    Add 'adds_as_modification'.

- Julian
Received on 2013-03-14 03:16:04 CET