ok to extend svn:externals syntax? -- was: Re: AW: [PATCH] commit --include-externals (v2)

[Neels J Hofmeyr <ne...@elego.de>]

On 11/10/2011 07:10 PM, C. Michael Pilato wrote:
> As a community, we need to decide how we will handle file externals in
> general.  Their clever implementation invites inconsistency.

I think there is general agreement (to the degree of common sense?) that 
file and dir externals should behave the same way.

> I prefer that file externals be treated the same way as directory externals
> in every behavior possible.  I find that easier to understand and account
> for myself, easier to explain to others.  But I don't know that the rest of
> the devs think the same way about them that I do.

I would be fine with keeping current trunk: it changes file externals' 
default behavior, so that they are treated like dir externals. So now it's: 
never include any externals in commit recursion, unless --include-externals.

But I think we still do need to add a way of making an external get included 
by default.

It was suggested to extend the svn:externals syntax, adding a flag that 
marks externals that should behave differently. By now this seems to me to 
be the best way out. What would that look like?

    [-rN] [-c] <URL>@P <PATH>

    -c = include in a recursive commit
    (positions of -r and -c exchangeable)

Does that sound like a good plan?

~Neels

Re: ok to extend svn:externals syntax? -- was: Re: AW: [PATCH] commit --include-externals (v2)

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

Neels J Hofmeyr wrote on Thu, Nov 10, 2011 at 21:34:30 +0100:
> It was suggested to extend the svn:externals syntax, adding a flag
> that marks externals that should behave differently. By now this
> seems to me to be the best way out. What would that look like?
> 
>    [-rN] [-c] <URL>@P <PATH>
> 
>    -c = include in a recursive commit
>    (positions of -r and -c exchangeable)
> 
> Does that sound like a good plan?

How does 1.7.0 behave when it runs into an externals property with such a value?

Re: ok to extend svn:externals syntax?

[Johan Corveleyn <jc...@gmail.com>]

On Sun, Nov 13, 2011 at 5:57 PM, Neels J Hofmeyr <ne...@elego.de> wrote:
> On 11/11/2011 11:41 AM, Julian Foad wrote:
>>
>> Neels J Hofmeyr wrote:
>>>
>>> I think there is general agreement (to the degree of common
>>> sense?) that file and dir externals should behave the same
>>> way.
>>
>> +1 to that.
>>
>>> I would be fine with keeping current trunk: it changes file
>>> externals' default behavior, so that they are treated like
>>> dir externals. So now it's: never include any externals in
>>> commit recursion, unless --include-externals.
>>
>> So far so good; this is definitely an improvement over the previous
>> mixture.
>
> Yea. This thread should probably have stopped there, actually. Here is the
> long reason why:
>
> I like your argument placing the whole issue in the bigger picture,
> mentioning the other aspects (propset, revert, add, delete...). Thinking
> about those brings me back to a general awkwardness with dir externals,
> which I have not mentioned before to keep things simpler; it all comes from
> the fact that a dir external has its own distinct WC-root & wc.db:
>
> Simple case: If I say 'svn commit sub/dir/target', 'svn' goes to
> 'sub/dir/target', finds its "most immediate" WC-root and uses that to find
> metadata (e.g. externals definitions). Say the immediate WC-root is 'dir'.
>
> Extend above case: assume the WC-root in 'dir' actually belongs to a dir
> external, and its defining svn:externals prop is set on 'sub', which is the
> "original" WC-root. The target path is still 'sub/dir/target'.
>
> To distinguish between above two cases is not so trivial. If 'svn' gets
> passed a target path and finds its WC-root, the only way of knowing whether
> that is already the "original", top-most WC-root is to scan upwards for more
> WC-roots. If we find a WC-root above it, then even that WC-root may itself
> be an external checked out by another parenting WC-root.
>
> Furthermore, imagine a "skip-nesting" like this:
>  WC-root (0) --> dir-external (1) --> dir-external (2)
> The deeper dir-external (2) may actually be defined in (0)! I.e. an
> svn:externals definition for any given WC-root may be found on *any* level
> of WC-root above itself. Only when there is *no* WC-root found all the way
> to the file system root that has an externals definition for the target in
> question can we be sure that the target is not part of an external. Note
> that (1) treats (2) like an unversioned dir, because it does not know about
> the definition in (0). Only if a target path is part of (0) will svn treat
> (2) as the external that it is.
>
> Note that the *target path* is of relevance, not the CWD.
>
> This makes a general comprehensive-external-modes approach expensive:
> *Every* affected command has to scan upwards all the way to the FS root to
> find potential WC-roots with externals definitions. Now that's awkward.
>
> Under this aspect, it would make sense to combine dir externals' wc.dbs with
> their topmost WC-root, i.e. to always automatically store sub-WCs' metadata
> in their single ancestral WC-root. (Queue Bert)

Yeah, I had the same thought over the weekend. If you're looking at
the bigger picture, things like this start to stand out. I'm not an
expert at all on (the history of) externals, but it's obvious that the
fact that a dir-external is currently almost the same as an embedded
working copy is becoming awkward for doing more advanced stuff.

> Another solution would be to mark each WC-root that is a dir external
> explicitly with its defining WC-root, at the time that the external is being
> checked out. (A simple WC-id <--> parent WC-root abspath table)
>
> BTW, only then can --include-externals work as expected with "skip-nested"
> WCs (the situation above where "(0) defines (2)"), and only then can we
> prevent the user from committing an external marked "read-only third-party"
> when the user passes it as explicit target (which may happen by accident
> when using wildcards, for example).

Yep, or when you're cd'ing into the external, and committing from
there (the known issue of not being able to block commits from a
revision-pegged dir-external: if you're inside it, you don't know that
you're revision-pegged). So if we want our users to think of it as a
"read-only third-party" external, we better fix this behavior, so
first find a solution to this problem (a dir-external should be
different from an embedded working copy with some lipstick on --- or
the lipstick better be very good).

> So I guess a really good externals implementation is still some way further
> down the road. Looking at it in Julian's bigger picture, I think I'm not up
> to that ATM...
>
> Let me mention that I have thankfully noticed Johan's and Stefan's remarks
> (prefer separate flag instead of using just revision peg, and not use '-c'
> because it means '-rN:M'), and that I agree with both.
>
> Thanks everyone for the discussion! Heh, threads like this always remind me
> of Greg, some time back, telling me to stop spinning my wheels :)

I don't know what discussions took place about this in the past, but
maybe it's time to start looking at the bigger picture, and to start
thinking about externals-ng or something. Maybe not for 1.8, but 1.9
or later? (note: I'm not volunteering, just making some noise actually
-- sorry).

Other random thoughts:

- Sometimes it would be cool if the server knew about externals. So
you can just use the "links" in urls, or browse them with ls, ...
Full-fledged symlinks, so to speak.

- Maybe there should be an explicit difference between "intra-repos
externals" and "inter-repos externals". There is a lot of behavior
that's interesting for externals pointing to the same repository, that
is less interesting for really external externals (e.g. recursive
propset, including in commit, branching/tagging together with the
parent, ...).

- Non-revision-pegged externals that point to something inside the
same branch-root (if we ever invent such a concept) should be
branched/tagged together with the parent-wc. Only for relative
externals, or also for absolute?

- Externals relative to the branch-root, or referring to other
branch-roots by name in some namespace (if we ever have such a
feature).

- What to do with branching/tagging for non-revision-pegged externals
that point outside the branch-root, and to something that does not
belong to the same "branch-context" (again, if we ever have such a
concept), e.g. a link to trunk of another component/library/...

- How about merge support in relation to externals? I don't know too
much about it, but there should be a clear specification on the
interaction of merge and externals.

- Depth support for externals [1] (sometimes you can almost use them
as "viewspecs", defining the layout of a sparse working copy ---
except that you can't specify depth). Or should something like this be
part of a grown-up viewspec feature, which can then have interaction
with externals (like: you can include a "viewspec-definition" in you
external definition)?

- Depth support for externals, part 2: let the specified depth (on
'svn up' or 'svn co') have an effect on the externals that are the
target of the command. [2]


[1] http://subversion.tigris.org/issues/show_bug.cgi?id=3216 -
externals with --depth
[2] http://subversion.tigris.org/issues/show_bug.cgi?id=3311 -
externals are not created unless depth=infinity


-- 
Johan

Re: ok to extend svn:externals syntax?

[Neels J Hofmeyr <ne...@elego.de>]

On 11/11/2011 11:41 AM, Julian Foad wrote:
> Neels J Hofmeyr wrote:
>> I think there is general agreement (to the degree of common
>> sense?) that file and dir externals should behave the same
>> way.
>
> +1 to that.
>
>> I would be fine with keeping current trunk: it changes file
>> externals' default behavior, so that they are treated like
>> dir externals. So now it's: never include any externals in
>> commit recursion, unless --include-externals.
>
> So far so good; this is definitely an improvement over the previous mixture.

Yea. This thread should probably have stopped there, actually. Here is the 
long reason why:

I like your argument placing the whole issue in the bigger picture,
mentioning the other aspects (propset, revert, add, delete...). Thinking 
about those brings me back to a general awkwardness with dir externals, 
which I have not mentioned before to keep things simpler; it all comes from 
the fact that a dir external has its own distinct WC-root & wc.db:

Simple case: If I say 'svn commit sub/dir/target', 'svn' goes to 
'sub/dir/target', finds its "most immediate" WC-root and uses that to find 
metadata (e.g. externals definitions). Say the immediate WC-root is 'dir'.

Extend above case: assume the WC-root in 'dir' actually belongs to a dir 
external, and its defining svn:externals prop is set on 'sub', which is the 
"original" WC-root. The target path is still 'sub/dir/target'.

To distinguish between above two cases is not so trivial. If 'svn' gets 
passed a target path and finds its WC-root, the only way of knowing whether 
that is already the "original", top-most WC-root is to scan upwards for more 
WC-roots. If we find a WC-root above it, then even that WC-root may itself 
be an external checked out by another parenting WC-root.

Furthermore, imagine a "skip-nesting" like this:
   WC-root (0) --> dir-external (1) --> dir-external (2)
The deeper dir-external (2) may actually be defined in (0)! I.e. an 
svn:externals definition for any given WC-root may be found on *any* level 
of WC-root above itself. Only when there is *no* WC-root found all the way 
to the file system root that has an externals definition for the target in 
question can we be sure that the target is not part of an external. Note 
that (1) treats (2) like an unversioned dir, because it does not know about 
the definition in (0). Only if a target path is part of (0) will svn treat 
(2) as the external that it is.

Note that the *target path* is of relevance, not the CWD.

This makes a general comprehensive-external-modes approach expensive: 
*Every* affected command has to scan upwards all the way to the FS root to 
find potential WC-roots with externals definitions. Now that's awkward.

Under this aspect, it would make sense to combine dir externals' wc.dbs with 
their topmost WC-root, i.e. to always automatically store sub-WCs' metadata 
in their single ancestral WC-root. (Queue Bert)

Another solution would be to mark each WC-root that is a dir external 
explicitly with its defining WC-root, at the time that the external is being 
checked out. (A simple WC-id <--> parent WC-root abspath table)

BTW, only then can --include-externals work as expected with "skip-nested" 
WCs (the situation above where "(0) defines (2)"), and only then can we 
prevent the user from committing an external marked "read-only third-party" 
when the user passes it as explicit target (which may happen by accident 
when using wildcards, for example).

So I guess a really good externals implementation is still some way further 
down the road. Looking at it in Julian's bigger picture, I think I'm not up 
to that ATM...

Let me mention that I have thankfully noticed Johan's and Stefan's remarks 
(prefer separate flag instead of using just revision peg, and not use '-c' 
because it means '-rN:M'), and that I agree with both.

Thanks everyone for the discussion! Heh, threads like this always remind me 
of Greg, some time back, telling me to stop spinning my wheels :)

~Neels

Re: ok to extend svn:externals syntax? -- was: Re: AW: [PATCH] commit --include-externals (v2)

[Julian Foad <ju...@btopenworld.com>]

Neels J Hofmeyr wrote:
> I think there is general agreement (to the degree of common
> sense?) that file and dir externals should behave the same
> way.

+1 to that.

> I would be fine with keeping current trunk: it changes file
> externals' default behavior, so that they are treated like
> dir externals. So now it's: never include any externals in
> commit recursion, unless --include-externals.

So far so good; this is definitely an improvement over the previous mixture.

> But I think we still do need to add a way of making an
> external get included by default.

Now we're getting into the subject of people using externals for different purposes.  Sometimes as a "view spec" to connect other subtrees of their own repository into the WC to make (as seamlessly as possible) a single WC so that they can edit and commit it all together.  Sometimes as a way to place a copy of some "third-party" source code into their working copy, but in a read-only manner in terms of version control: they don't intend to commit to it.

> It was suggested to extend the svn:externals syntax, adding
> a flag that marks externals that should behave differently.
> By now this seems to me to be the best way out. What would
> that look like?  [suggestion snipped]

The real-world issue is that there are different uses for externals, and one set of behaviour is not appropriate for all uses.  In particular, there's a "read-only" usage and a "part of my working copy editing space" usage.  Maybe we should not jump straight from the real-world issue to a switch that's designed to control just one aspect of Subversion's behaviour.  That might not really be a solution to the problem.  I don't want to see us extending the "externals" feature with a new little knob every year, and then telling users that they can certainly get the result they need, all they have to do is remember to set the magic incantation "x=N y=0 z=yes" every time.

It's hard work for us to think and define two or three categories of real-world uses that will best fit the ways people want externals to behave, but if we do that we will be providing users with a useful machine and not just a collection of parts.

If a user doesn't want "commit" to recurse into their checkout of some third-party source code, then they probably don't want "svn propset -R" or "svn revert -R" to recurse into it either.  In fact they might even like the files in that sub-tree to be marked read-only on disk.  What I'm saying is there's a set of specific behaviours that all come together as a collective way to treat "a subtree of third-party files".  Each individual aspect of that behaviour, such as whether to make the files read-only on disk, should be designed by us as part of a coherent whole behaviour.  (If that setting needs to be user-controllable, then it should probably be in e.g. the per-user config, not a switch in the "external" definition.)

So what's my suggestion?

  * Two new modes, each signified by some switch in the external definition.

  * "read-only third-party" mode -- Exclude this external from recursive commit, propset, add, delete, revert, etc. Maybe also warn if targets in this external are specified in such commands along with targets in the parent working copy, because such specifications are likely to be accidental. As a future enhancement, maybe make the checked-out files read-only on disk a bit like svn:needs-lock does.

  * "full participation working-copy" mode -- Include in recursive commits and modification commands (propset, add, revert, etc.).

  * If neither mode is specified in the external definition, use the default behaviour which is the current trunk behaviour, perhaps with further changes if we decide to make the default better or more self-consistent at the expecnse of backward compatibility.


The principle is that the user should choose the general mode, and then any specific enhancements to the way Subversion handles that mode should be obtained just by updating the client software and not by requiring the user to add more switches into the external definitions.  In other words, I think users will appreciate us giving them just a simple mode selector described in high-level task-focused (not vague) terms, and thinking centrally (here in the dev community) about exactly what aspects of Subversion's behaviour will best work together to facilitate those tasks.

- Julian

Re: ok to extend svn:externals syntax? -- was: Re: AW: [PATCH] commit --include-externals (v2)

[Neels J Hofmeyr <ne...@elego.de>]

On 11/10/2011 09:39 PM, C. Michael Pilato wrote:
> The next change we make to the externals syntax needs to be to add an
> explicit "#format = 3" header to it so we can stop trying to deduce the
> format the user intended!

now that's cumbersome. a footer would be much nicer. ;)

~Neels

Re: ok to extend svn:externals syntax? -- was: Re: AW: [PATCH] commit --include-externals (v2)

["C. Michael Pilato" <cm...@collab.net>]

On 11/10/2011 03:34 PM, Neels J Hofmeyr wrote:
> It was suggested to extend the svn:externals syntax, adding a flag that
> marks externals that should behave differently. By now this seems to me to
> be the best way out. What would that look like?

The next change we make to the externals syntax needs to be to add an
explicit "#format = 3" header to it so we can stop trying to deduce the
format the user intended!

Beyond that, I'll reserve comment about specific format changes and the
general question you are asking.  :-)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Re: ok to extend svn:externals syntax? -- was: Re: AW: [PATCH] commit --include-externals (v2)

[Stefan Sperling <st...@elego.de>]

On Thu, Nov 10, 2011 at 09:34:30PM +0100, Neels J Hofmeyr wrote:
> It was suggested to extend the svn:externals syntax, adding a flag
> that marks externals that should behave differently. By now this
> seems to me to be the best way out. What would that look like?
> 
>    [-rN] [-c] <URL>@P <PATH>
> 
>    -c = include in a recursive commit
>    (positions of -r and -c exchangeable)
> 
> Does that sound like a good plan?

-c means -rN-1:N on the command line.
Please let's not reuse the -c flag for a different purpose.

I would prefer something like this:

  [-rN] [--always-commit] <URL>@P <PATH>
 
  --always-commit = include in a recursive commit
  (positions of -r and --always-commit exchangeable)