You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@wandisco.com> on 2010/11/24 10:41:46 UTC
Re: svn commit: r1037738 - "Summary of updates"
On Mon, 2010-11-22, cmpilato@apache.org wrote:
> Issue #3693 - Multi-target update output is quite confusing.
>
> Add notifications at the start of each performed update, and a summary
> at the end of the whole mess.
Hi Mike. I like the direction of this.
Sometimes the summary prints the abspath instead of the relpath
specified on the command line. Specifically, this happens if you
specify a changelist. However, the "Updating..." line manages to print
the relpath. In the following transcript, the debug marked (1) shows
targets[0] near the start of svn_cl__update(), and (2) shows that
targets[0] has been converted to an abspath after changelist processing.
[[[
$ ~/src/subversion-c/subversion/tests/cmdline/changelist_tests.py
--bin=/home/julianfoad/src/subversion-c/bin 8
PASS: changelist_tests.py 8: update --changelist
julianfoad@edith ~/src/subversion-c/obj-dir/subversion/tests/cmdline
$ svn up -r1 svn-test-work/working_copies/changelist_tests-8/A/muDBG:
update-cmd.c: 95: (1)
'svn-test-work/working_copies/changelist_tests-8/A/mu'
DBG: update-cmd.c: 133: (2)
'svn-test-work/working_copies/changelist_tests-8/A/mu'
Updating 'svn-test-work/working_copies/changelist_tests-8/A/mu' ...
U svn-test-work/working_copies/changelist_tests-8/A/mu
Updated to revision 1.
julianfoad@edith ~/src/subversion-c/obj-dir/subversion/tests/cmdline
$ svn up --cl u svn-test-work/working_copies/changelist_tests-8/DBG:
update-cmd.c: 95: (1) 'svn-test-work/working_copies/changelist_tests-8'
DBG: update-cmd.c: 133: (2)
'/home/julianfoad/build/subversion-c/subversion/tests/cmdline/svn-test-work/working_copies/changelist_tests-8/A/mu'
Updating 'svn-test-work/working_copies/changelist_tests-8/A/mu' ...
U svn-test-work/working_copies/changelist_tests-8/A/mu
Updated to revision 2.
Updating 'svn-test-work/working_copies/changelist_tests-8/A/D/G/tau' ...
Updated to revision 2.
Summary of updates:
Updated
'/home/julianfoad/build/subversion-c/subversion/tests/cmdline/svn-test-work/working_copies/changelist_tests-8/A/mu' to r2.
Updated
'/home/julianfoad/build/subversion-c/subversion/tests/cmdline/svn-test-work/working_copies/changelist_tests-8/A/D/G/tau' to r2.
]]]
- Julian
Re: svn commit: r1037738 - "Summary of updates"
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Nov 29, 2010 at 02:17:13PM -0500, C. Michael Pilato wrote:
> Not necessarily anything more. I mean, test expectations reflect, to some
> degree, the expectations of Subversion users wrapping the command-line for
> output parsing. I sense we're cool with some sane amount of change in that
> output so as to make the tool better without completely breaking the world.
> :-) ('Cause if we're cool with breaking the world, I have quite a few
> changes in mind that I'd like to make to the output!)
I've said this before, but, yes, break the world.
I'd say just propose these changes. I think we can afford to break compat in
the CLI output. The CLI is for interactive use. People can script against
it, sure. But they should be using XML output where possible, or use the
bindings, if they rely on their scripts for production rather than quick
run-once-and-throw-away hacks.
Stefan
Re: svn commit: r1037738 - "Summary of updates"
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Mon, Nov 29, 2010 at 14:17:13 -0500:
> On 11/26/2010 01:01 AM, Daniel Shahaf wrote:
> > Thanks for this :-). It works as expected when the current directory is
> > entered by its name, but not when it's entered through a symlink:
> >
> > [[[
> > % cd /tmp
> >
> > % ln -s wc1 wcalias
> >
> > % cd wc1
> >
> > % $svn up /tmp/wc1/trunk/iota
> > Updating 'trunk/iota' ...
> >
> > % cd /tmp/wcalias
> >
> > % $svn up /tmp/wc1/trunk/iota11D
> > Updating 'trunk/iota' ...
> >
> > % $svn up /tmp/wcalias/trunk/iota
> > Updating '/tmp/wcalias/trunk/iota' ...
> > ]]]
> >
> >
> > Ideally, the last output would have used the relative path 'trunk/iota',
> > too. However, I'm not sure how to easily solve that --- is it as easy
> > as calling some "resolve symlinks" function on the absolute-cwd string?
>
> Do we have a "resolve symlinks" function?
I don't see anything in APR or in svn_io.h. Can we use realpath(3)? Or
should we roll our own?
Daniel
(I rolled a prototype here)
Re: svn commit: r1037738 - "Summary of updates"
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/26/2010 01:01 AM, Daniel Shahaf wrote:
> Thanks for this :-). It works as expected when the current directory is
> entered by its name, but not when it's entered through a symlink:
>
> [[[
> % cd /tmp
>
> % ln -s wc1 wcalias
>
> % cd wc1
>
> % $svn up /tmp/wc1/trunk/iota
> Updating 'trunk/iota' ...
>
> % cd /tmp/wcalias
>
> % $svn up /tmp/wc1/trunk/iota11D
> Updating 'trunk/iota' ...
>
> % $svn up /tmp/wcalias/trunk/iota
> Updating '/tmp/wcalias/trunk/iota' ...
> ]]]
>
>
> Ideally, the last output would have used the relative path 'trunk/iota',
> too. However, I'm not sure how to easily solve that --- is it as easy
> as calling some "resolve symlinks" function on the absolute-cwd string?
Do we have a "resolve symlinks" function?
>> I'm wondering what would happen -- that is, how
>> much chaos would ensue -- if we changed subversion/svn/notify.c:notify() to,
>> in the case where n->path is not a URL, always first convert n->path to an
>> absolute path before doing the prefix removal stuff.
>
> What chaos could ensure? Just N+1 test expectations, or something more?
Not necessarily anything more. I mean, test expectations reflect, to some
degree, the expectations of Subversion users wrapping the command-line for
output parsing. I sense we're cool with some sane amount of change in that
output so as to make the tool better without completely breaking the world.
:-) ('Cause if we're cool with breaking the world, I have quite a few
changes in mind that I'd like to make to the output!)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1037738 - "Summary of updates"
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Thu, Nov 25, 2010 at 10:33:37 -0500:
> On 11/25/2010 05:07 AM, Daniel Shahaf wrote:
> > C. Michael Pilato wrote on Wed, Nov 24, 2010 at 13:42:09 -0500:
> >> when the targets are in or under ${CWD}, absolute paths otherwise, which
> >> is the typical behavior of the command-line client notification code.)
> >
> > But what if the user specified a target, which is under `pwd`, using an
> > absolute path? Do we want to notify this using an absolute path or
> > a relative path?
> >
> > If we go with "make it absolute whenever it's outside cwd", then I think
> > that (as you suggest, but not necessarily as it works right now) it
> > makes sense to also do the opposite --- convert abspath to cwd-relative
> > path if they happen to be under cwd.
>
> I agree. I (think) I've done this in r1039072.
>
Thanks for this :-). It works as expected when the current directory is
entered by its name, but not when it's entered through a symlink:
[[[
% cd /tmp
% ln -s wc1 wcalias
% cd wc1
% $svn up /tmp/wc1/trunk/iota
Updating 'trunk/iota' ...
% cd /tmp/wcalias
% $svn up /tmp/wc1/trunk/iota11D
Updating 'trunk/iota' ...
% $svn up /tmp/wcalias/trunk/iota
Updating '/tmp/wcalias/trunk/iota' ...
]]]
Ideally, the last output would have used the relative path 'trunk/iota',
too. However, I'm not sure how to easily solve that --- is it as easy
as calling some "resolve symlinks" function on the absolute-cwd string?
> >> 2. Should "Skipped" items follow suit? (I say, "yes").
> >
> > +1
>
> I did *not* do this yet. Right now, we get a mix of absolute and relative
> paths passed through the notification system by client layer. I'm not
> convinced (at all!) that this "mix" has been well-thought-out. I suspect
> it's just in whatever state naturally evolved from our internal conversion
> to abspaths everywhere.
> I'm wondering what would happen -- that is, how
> much chaos would ensue -- if we changed subversion/svn/notify.c:notify() to,
> in the case where n->path is not a URL, always first convert n->path to an
> absolute path before doing the prefix removal stuff.
>
What chaos could ensure? Just N+1 test expectations, or something more?
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1037738 - "Summary of updates"
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/25/2010 05:07 AM, Daniel Shahaf wrote:
> C. Michael Pilato wrote on Wed, Nov 24, 2010 at 13:42:09 -0500:
>> when the targets are in or under ${CWD}, absolute paths otherwise, which
>> is the typical behavior of the command-line client notification code.)
>
> But what if the user specified a target, which is under `pwd`, using an
> absolute path? Do we want to notify this using an absolute path or
> a relative path?
>
> If we go with "make it absolute whenever it's outside cwd", then I think
> that (as you suggest, but not necessarily as it works right now) it
> makes sense to also do the opposite --- convert abspath to cwd-relative
> path if they happen to be under cwd.
I agree. I (think) I've done this in r1039072.
>> 2. Should "Skipped" items follow suit? (I say, "yes").
>
> +1
I did *not* do this yet. Right now, we get a mix of absolute and relative
paths passed through the notification system by client layer. I'm not
convinced (at all!) that this "mix" has been well-thought-out. I suspect
it's just in whatever state naturally evolved from our internal conversion
to abspaths everywhere. I'm wondering what would happen -- that is, how
much chaos would ensue -- if we changed subversion/svn/notify.c:notify() to,
in the case where n->path is not a URL, always first convert n->path to an
absolute path before doing the prefix removal stuff.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1037738 - "Summary of updates"
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Wed, Nov 24, 2010 at 13:42:09 -0500:
> On 11/24/2010 01:12 PM, Julian Foad wrote:
> >> C. Michael Pilato wrote on Wed, Nov 24, 2010 at 10:36:31 -0500:
> >>> On 11/24/2010 05:41 AM, Julian Foad wrote:
> >>>> Sometimes the summary prints the abspath instead of the relpath
> >>>> specified on the command line. Specifically, this happens if you
> >>>> specify a changelist.
> >
> > C-Mike wrote:
> >> r1038650
> >
> > Thanks, that looks good.
> >
> > Daniel Shahaf wrote:
> >> Another case (with current HEAD, i.e., after the changelist patch):
> >>
> >> % $svn up ../t2
> >> Updating '/home/daniel/src/svn/t2' ...
> >> ^C
> >
> > That's a slightly different issue, the handling of "..". A more
> > comprehensive transcript:
> >
> > $ svn up ../readme.txt ../subversion-b
> > Skipped '../readme.txt'
> > Updating '/home/julianfoad/src/subversion-b' ...
> > G /home/julianfoad/src/subversion-b/subversion/libsvn_subr/dirent_uri.c
> > Updated to revision 1038733.
> > Summary of updates:
> > Updated '../subversion-b' to r1038733.
> > Summary of conflicts:
> > Skipped paths: 1
>
> So, I have a pair of questions:
>
> 1. Given the option of displaying either "../subversion-b" or
> "/home/julianfoad/src/subversion-b", which is preferred? NOTE that we
In general I think it's nice to preserve the relative path.
> probably do *not* reasonably have that choice because at some point
> we'll have converted into an abspath internally, and IMO it just isn't
> worth it to pass around both an original path-as-the-user-specified-it
> and the normalized abspath. (I believe we should use relative paths
I'm not sure where the conversion to abspath occurs relative to
the generation of notifications, but I see what you mean.
> when the targets are in or under ${CWD}, absolute paths otherwise, which
> is the typical behavior of the command-line client notification code.)
>
But what if the user specified a target, which is under `pwd`, using an
absolute path? Do we want to notify this using an absolute path or
a relative path?
If we go with "make it absolute whenever it's outside cwd", then I think
that (as you suggest, but not necessarily as it works right now) it
makes sense to also do the opposite --- convert abspath to cwd-relative
path if they happen to be under cwd.
> 2. Should "Skipped" items follow suit? (I say, "yes").
>
+1
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
Re: svn commit: r1037738 - "Summary of updates"
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/24/2010 01:12 PM, Julian Foad wrote:
>> C. Michael Pilato wrote on Wed, Nov 24, 2010 at 10:36:31 -0500:
>>> On 11/24/2010 05:41 AM, Julian Foad wrote:
>>>> Sometimes the summary prints the abspath instead of the relpath
>>>> specified on the command line. Specifically, this happens if you
>>>> specify a changelist.
>
> C-Mike wrote:
>> r1038650
>
> Thanks, that looks good.
>
> Daniel Shahaf wrote:
>> Another case (with current HEAD, i.e., after the changelist patch):
>>
>> % $svn up ../t2
>> Updating '/home/daniel/src/svn/t2' ...
>> ^C
>
> That's a slightly different issue, the handling of "..". A more
> comprehensive transcript:
>
> $ svn up ../readme.txt ../subversion-b
> Skipped '../readme.txt'
> Updating '/home/julianfoad/src/subversion-b' ...
> G /home/julianfoad/src/subversion-b/subversion/libsvn_subr/dirent_uri.c
> Updated to revision 1038733.
> Summary of updates:
> Updated '../subversion-b' to r1038733.
> Summary of conflicts:
> Skipped paths: 1
So, I have a pair of questions:
1. Given the option of displaying either "../subversion-b" or
"/home/julianfoad/src/subversion-b", which is preferred? NOTE that we
probably do *not* reasonably have that choice because at some point
we'll have converted into an abspath internally, and IMO it just isn't
worth it to pass around both an original path-as-the-user-specified-it
and the normalized abspath. (I believe we should use relative paths
when the targets are in or under ${CWD}, absolute paths otherwise, which
is the typical behavior of the command-line client notification code.)
2. Should "Skipped" items follow suit? (I say, "yes").
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1037738 - "Summary of updates"
Posted by Julian Foad <ju...@wandisco.com>.
> C. Michael Pilato wrote on Wed, Nov 24, 2010 at 10:36:31 -0500:
> > On 11/24/2010 05:41 AM, Julian Foad wrote:
> > > Sometimes the summary prints the abspath instead of the relpath
> > > specified on the command line. Specifically, this happens if you
> > > specify a changelist.
C-Mike wrote:
> r1038650
Thanks, that looks good.
Daniel Shahaf wrote:
> Another case (with current HEAD, i.e., after the changelist patch):
>
> % $svn up ../t2
> Updating '/home/daniel/src/svn/t2' ...
> ^C
That's a slightly different issue, the handling of "..". A more
comprehensive transcript:
$ svn up ../readme.txt ../subversion-b
Skipped '../readme.txt'
Updating '/home/julianfoad/src/subversion-b' ...
G /home/julianfoad/src/subversion-b/subversion/libsvn_subr/dirent_uri.c
Updated to revision 1038733.
Summary of updates:
Updated '../subversion-b' to r1038733.
Summary of conflicts:
Skipped paths: 1
- Julian
Re: svn commit: r1037738 - "Summary of updates"
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
C. Michael Pilato wrote on Wed, Nov 24, 2010 at 10:36:31 -0500:
> On 11/24/2010 05:41 AM, Julian Foad wrote:
> > On Mon, 2010-11-22, cmpilato@apache.org wrote:
> >> Issue #3693 - Multi-target update output is quite confusing.
> >>
> >> Add notifications at the start of each performed update, and a summary
> >> at the end of the whole mess.
> >
> > Hi Mike. I like the direction of this.
> >
> > Sometimes the summary prints the abspath instead of the relpath
> > specified on the command line. Specifically, this happens if you
> > specify a changelist.
>
Another case (with current HEAD, i.e., after the changelist patch):
% $svn up ../t2
Updating '/home/daniel/src/svn/t2' ...
^C
> Thanks, Julian. I'll look into this.
>
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
Re: svn commit: r1037738 - "Summary of updates"
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/24/2010 10:36 AM, C. Michael Pilato wrote:
> On 11/24/2010 05:41 AM, Julian Foad wrote:
>> On Mon, 2010-11-22, cmpilato@apache.org wrote:
>>> Issue #3693 - Multi-target update output is quite confusing.
>>>
>>> Add notifications at the start of each performed update, and a summary
>>> at the end of the whole mess.
>>
>> Hi Mike. I like the direction of this.
>>
>> Sometimes the summary prints the abspath instead of the relpath
>> specified on the command line. Specifically, this happens if you
>> specify a changelist.
>
> Thanks, Julian. I'll look into this.
r1038650
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1037738 - "Summary of updates"
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/24/2010 05:41 AM, Julian Foad wrote:
> On Mon, 2010-11-22, cmpilato@apache.org wrote:
>> Issue #3693 - Multi-target update output is quite confusing.
>>
>> Add notifications at the start of each performed update, and a summary
>> at the end of the whole mess.
>
> Hi Mike. I like the direction of this.
>
> Sometimes the summary prints the abspath instead of the relpath
> specified on the command line. Specifically, this happens if you
> specify a changelist.
Thanks, Julian. I'll look into this.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand