Re: BUG: Delete svn:special property on symlink; hilarity ensues.

[Daniel Shahaf <d....@daniel.shahaf.name>]

Karl Fogel wrote on Fri, 31 Aug 2018 14:06 -0500:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
> >This is exactly how one adds or edits a symlink on !HAVE_SYMLINK
> >platforms, including Windows.  (You can get this behaviour on Unix if
> >you get configure to lie about the HAVE_SYMLINK test result.)  I suppose
> >it's a public API therefore, although it's a bit odd that our
> >serialization format _is_ our public API; I'd have expected some sort of
> >layering decoupling the two.
> 
> Well, I don't mind the part about our serialization format also being 
> effectively our API for this.
> 
> But the question is, what *should* the behavior be if one deletes the 
> 'svn:special' property of a symlink?
> 
>   $ svn propdel svn:special some_version_controlled_symlink
>   $ svn status -q
>   ~M      some_version_controlled_symlink
>   $ svn commit -m "This commit will fail."
>   svn: E145001: Commit failed (details follow):
>   svn: E145001: Node '/.../some_version_controlled_symlink has \
>                 unexpectedly changed kind
>   $ 
> 
> The above doesn't seem right...
> 

As far as the FS layer is concerned, sure.  In that layer, svn:special
is just another property, with no special semantics, behaving exactly as
in your proposal.  (No pun intended)

However, in higher layers, that do know of svn:special's semantics, I
don't know if it makes sense to allow a node to change type from "file"
to "symlink".  Note that we added svn_node_symlink to svn_node_kind_t
(in 1.8) as a distinct value; the thinking then, IIRC, was to move
towards symlinks as first-class citizens.

> I think my original proposal is a sane outcome, or is maybe the least 
> insane outcome.  But I'd like to know what others think.
> 

That proposal is certainly simple and predictable.  However, I'd like to
spell out an alternative proposal, driven by "nodes shouldn't change
types" thinking:

1. Retain the ability to add special files by writing their
repository-normal contents to a file and doing 'svn add foo; svn ps
svn:special yes foo'.

2. Have 'svn propdel svn:special' error out with "use 'svn rm' instead".

3. Disallow changing the svn:special type of a file (such as from "link"
to something else).

Rationale (#1): This is the canonical way to add symlinks on windows.  This
is also the canonical forward-compatible way to add, using a 1.10
client, any svn:special types (besides SVN_SUBST__SPECIAL_LINK_STR) we
might invent in 1.11.

Rationale (#2 and #3): nodes shouldn't change types.  In #2 and #3 we
might want to have a --force-changing-svn:special-type escape hatch,
allowing the error to be overruled, for forward compatibility.

The case of 'svn propset svn:special yes' of a file that isn't a
locally-added-without-history one would behave similarly.

I'm not sure yet which proposal I'd back, by the way.  The above is just
out-loud thinking.

Cheers,

Daniel
(stsp: I've started drafting a reply but haven't finished yet...)

Re: BUG: Delete svn:special property on symlink; hilarity ensues.

[Julian Foad <ju...@foad.me.uk>]

Branko Čibej wrote:
> The only correct way to change a node's type is to replace the node itself.
The crucial missing piece of the definition is the mapping between the client-side notions of node kinds and node lifetimes and their server-side counterparts. (Where client side treats specials as different kinds, and server doesn't.)

We need to define that mapping and then not only the behaviours of propset and propdel will follow, but also of for example "log" and other history-following operations.
- Julian

Re: BUG: Delete svn:special property on symlink; hilarity ensues.

[Branko Čibej <br...@apache.org>]

On 01.09.2018 21:14, Daniel Shahaf wrote:
> Karl Fogel wrote on Fri, 31 Aug 2018 14:06 -0500:
>> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>>> This is exactly how one adds or edits a symlink on !HAVE_SYMLINK
>>> platforms, including Windows.  (You can get this behaviour on Unix if
>>> you get configure to lie about the HAVE_SYMLINK test result.)  I suppose
>>> it's a public API therefore, although it's a bit odd that our
>>> serialization format _is_ our public API; I'd have expected some sort of
>>> layering decoupling the two.
>> Well, I don't mind the part about our serialization format also being 
>> effectively our API for this.
>>
>> But the question is, what *should* the behavior be if one deletes the 
>> 'svn:special' property of a symlink?
>>
>>   $ svn propdel svn:special some_version_controlled_symlink
>>   $ svn status -q
>>   ~M      some_version_controlled_symlink
>>   $ svn commit -m "This commit will fail."
>>   svn: E145001: Commit failed (details follow):
>>   svn: E145001: Node '/.../some_version_controlled_symlink has \
>>                 unexpectedly changed kind
>>   $ 
>>
>> The above doesn't seem right...


Sure it does. We could forbid deleting the svn:special property in the
command-line client, but that would just be papering over the underlying
issue.

The only correct way to change a node's type is to replace the node itself.

I admit it could be nice to stop the user from accidentally doing
something that would make the WC uncommittable. We're already safe as
far as the repository is concerned, so, patches welcome to change the
client-side UI.

-- Brane

Re: BUG: Delete svn:special property on symlink; hilarity ensues.

[Karl Fogel <kf...@red-bean.com>]

Daniel Shahaf <d....@daniel.shahaf.name> writes:
>However, in higher layers, that do know of svn:special's semantics, I
>don't know if it makes sense to allow a node to change type from "file"
>to "symlink".  Note that we added svn_node_symlink to svn_node_kind_t
>(in 1.8) as a distinct value; the thinking then, IIRC, was to move
>towards symlinks as first-class citizens.

I think you're right, and this matches well with what Brane said in this thread too:

>The only correct way to change a node's type is to replace the node itself.

So, on to your proposal:

>That proposal is certainly simple and predictable.  However, I'd like to
>spell out an alternative proposal, driven by "nodes shouldn't change
>types" thinking:
>
>1. Retain the ability to add special files by writing their
>repository-normal contents to a file and doing 'svn add foo; svn ps
>svn:special yes foo'.
>
>2. Have 'svn propdel svn:special' error out with "use 'svn rm' instead".
>
>3. Disallow changing the svn:special type of a file (such as from "link"
>to something else).

I like that better than my proposal.

>Rationale (#1): This is the canonical way to add symlinks on windows.  This
>is also the canonical forward-compatible way to add, using a 1.10
>client, any svn:special types (besides SVN_SUBST__SPECIAL_LINK_STR) we
>might invent in 1.11.
>
>Rationale (#2 and #3): nodes shouldn't change types.  In #2 and #3 we
>might want to have a --force-changing-svn:special-type escape hatch,
>allowing the error to be overruled, for forward compatibility.
>
>The case of 'svn propset svn:special yes' of a file that isn't a
>locally-added-without-history one would behave similarly.
>
>I'm not sure yet which proposal I'd back, by the way.  The above is just
>out-loud thinking.

I agree with all of this.  As far as I'm concerned, this would be a fine behavior.  In the specific usage circumstances I was in, this behavior would have nudged me to do The Right Thing (namely, 'svn rm' and replace the node).

Basically, we're making "svn:special" be a property that users can't manipulate directly when the proper special behaviors are available on on the system.  IOW, if your system supports symlinks, then the user shouldn't try to do things to the "svn:special" property, but should instead do things to symlinks.

If the system doesn't support symlinks, then we fall back to allowing the user to do things to Subversion's underlying representation, because that's pretty much the only thing we can offer that allows the user to collaborate with users on symlink-supporting systems.

Best regards,
-Karl