You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Neels J Hofmeyr <ne...@elego.de> on 2012/11/16 17:20:50 UTC
duplicate externals cause update error -- thoughts?
Hi,
I've created a check during 'svn propset' that refuses to save svn:externals
props that contain the same target more than once. The result is an error,
same as with any other externals format violations. (#4227)
Turns out the same parsing code is (obviously) run during update. So in the
externals phase of the update, the update will bail if the server sends an
svn:externals with duplicate targets.
Is that bad? Is that good?
The standard update will complete. Only updates of the externals items will
interrupt. (And the other entries of that svn:externals prop will not be
handled until a later 'svn up'.)
Subsequently, the prop can be edited and the duplicate removed; an update
after that will complete the update properly. In short, it works just like
any other svn:externals format error that happens to come in from the repos.
At first I thought I would need to disable the dup check for update, i.e.
only enable it for the actual propset/propedit invocations. But now that I
realized the error is recoverable, I think an error message is not that bad
at all. Like this you are told that your external will be ambiguous and it
will not update externals unless you fixed it.
Another possibility is to simply print a warning and not bail...
(All of this is just about duplicates within one single svn:externals prop)
I'm not sure which way to go, so thanks for any thoughts.
~Neels
Re: AW: duplicate externals cause update error -- thoughts?
Posted by Neels J Hofmeyr <ne...@elego.de>.
On 2012-11-19 16:20, Julian Foad wrote:
> let the *caller* check for duplicates.
I was, actually, also thinking about that at some point... I guess that's
the best call after all. I'll limit to propset/propedit and provide a
function to do the check.
/me still working on it
~Neels
Re: AW: duplicate externals cause update error -- thoughts?
Posted by Julian Foad <ju...@btopenworld.com>.
Neels J Hofmeyr wrote:
> Thanks for your input, Markus!
>
> As so often with externals, things are more complex than one would think.
>
> - When an externals error occurs, the entire externals property remains
> unhandled. That needn't be so.
>
> - When an existing repos has duplicates, doing a simple update/checkout with
> the new error check would be tiresome, as one would first need to edit all
> those props before the update completes.
>
> - To be able to issue a warning, the parsing function needs a new argument
> with a callback function (or something). Like this it can notify about more
> than one duplicate. It could still parse everything and return the parsed
> data, and externals updating could continue to work the same (stupid) way it
> does now: fifo style. Just adding a flag to the function to select between
> error and warning won't work, as that function does not have any way to
> issue warnings.
>
> Right now I'm thinking:
> - add a callback arg to svn_wc_parse_externals_description3().
> - the callback fn usually tries to punch a warning thru to the user.
> - maybe during propset/propedit, that callback causes an error.
Sounds icky. Maybe that's the wrong level to detect duplicates. Instead, let this parse function return a list describing exactly what it parsed, and let the *caller* check for duplicates.
- Julian
> Maybe all the other externals parse errors should be handled in the same
> way? We'll see about that later. Maybe we should even ... reimplement
> externals from scratch ;)
Re: AW: duplicate externals cause update error -- thoughts?
Posted by Philip Martin <ph...@wandisco.com>.
Neels J Hofmeyr <ne...@elego.de> writes:
> - To be able to issue a warning, the parsing function needs a new argument
> with a callback function (or something). Like this it can notify about more
> than one duplicate. It could still parse everything and return the parsed
> data, and externals updating could continue to work the same (stupid) way it
> does now: fifo style. Just adding a flag to the function to select between
> error and warning won't work, as that function does not have any way to
> issue warnings.
Perhaps the caller could catch the error, issue the warning, and skip
processing that svn:externals. The caller already has notification
support.
svn_wc_parse_externals_description3 is also used in upgrade. What do we
do there? I suppose we could do the same, issue a warning and skip
processing the svn:externals.
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: AW: duplicate externals cause update error -- thoughts?
Posted by Neels J Hofmeyr <ne...@elego.de>.
Thanks for your input, Markus!
As so often with externals, things are more complex than one would think.
- When an externals error occurs, the entire externals property remains
unhandled. That needn't be so.
- When an existing repos has duplicates, doing a simple update/checkout with
the new error check would be tiresome, as one would first need to edit all
those props before the update completes.
- To be able to issue a warning, the parsing function needs a new argument
with a callback function (or something). Like this it can notify about more
than one duplicate. It could still parse everything and return the parsed
data, and externals updating could continue to work the same (stupid) way it
does now: fifo style. Just adding a flag to the function to select between
error and warning won't work, as that function does not have any way to
issue warnings.
Right now I'm thinking:
- add a callback arg to svn_wc_parse_externals_description3().
- the callback fn usually tries to punch a warning thru to the user.
- maybe during propset/propedit, that callback causes an error.
Maybe all the other externals parse errors should be handled in the same
way? We'll see about that later. Maybe we should even ... reimplement
externals from scratch ;)
~Neels
On 2012-11-19 08:17, Markus Schaber wrote:
> Could you give a flag to the parsing code whether it should error or just warn on that issue?
>
> So on an update, we can use the warning, while the code can error out on propset/propedit.
>
> And (of course), for backwards compatibility, the behavior in case of an collision should resemble the old behavior (which of the two entries wins...)
>
>
> Best regards
>
> Markus Schaber
>
> CODESYS(r) a trademark of 3S-Smart Software Solutions GmbH
>
> Inspiring Automation Solutions
>
> 3S-Smart Software Solutions GmbH
> Dipl.-Inf. Markus Schaber | Product Development Core Technology
> Memminger Str. 151 | 87439 Kempten | Germany
> Tel. +49-831-54031-979 | Fax +49-831-54031-50
>
> E-Mail: m.schaber@codesys.com | Web: http://www.codesys.com
> CODESYS internet forum: http://forum.codesys.com
>
> Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915>
>
AW: duplicate externals cause update error -- thoughts?
Posted by Markus Schaber <m....@codesys.com>.
Hi, Neels,
Von: Neels J Hofmeyr [mailto:neels@elego.de]
>
> I've created a check during 'svn propset' that refuses to save
> svn:externals props that contain the same target more than once. The
> result is an error, same as with any other externals format violations.
> (#4227)
>
> Turns out the same parsing code is (obviously) run during update. So in
> the externals phase of the update, the update will bail if the server
> sends an svn:externals with duplicate targets.
>
> Is that bad? Is that good?
>
> The standard update will complete. Only updates of the externals items
> will interrupt. (And the other entries of that svn:externals prop will not
> be handled until a later 'svn up'.)
>
> Subsequently, the prop can be edited and the duplicate removed; an update
> after that will complete the update properly. In short, it works just like
> any other svn:externals format error that happens to come in from the
> repos.
>
> At first I thought I would need to disable the dup check for update, i.e.
> only enable it for the actual propset/propedit invocations. But now that I
> realized the error is recoverable, I think an error message is not that
> bad at all. Like this you are told that your external will be ambiguous
> and it will not update externals unless you fixed it.
>
> Another possibility is to simply print a warning and not bail...
>
> (All of this is just about duplicates within one single svn:externals prop)
>
> I'm not sure which way to go, so thanks for any thoughts.
Could you give a flag to the parsing code whether it should error or just warn on that issue?
So on an update, we can use the warning, while the code can error out on propset/propedit.
And (of course), for backwards compatibility, the behavior in case of an collision should resemble the old behavior (which of the two entries wins...)
Best regards
Markus Schaber
CODESYS(r) a trademark of 3S-Smart Software Solutions GmbH
Inspiring Automation Solutions
3S-Smart Software Solutions GmbH
Dipl.-Inf. Markus Schaber | Product Development Core Technology
Memminger Str. 151 | 87439 Kempten | Germany
Tel. +49-831-54031-979 | Fax +49-831-54031-50
E-Mail: m.schaber@codesys.com | Web: http://www.codesys.com
CODESYS internet forum: http://forum.codesys.com
Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915>