[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Issue #3653: svn update should not output svn:external fetches if they have not been updated

From: Tijn Porcelijn <tijn.porcelijn_at_q-go.com>
Date: Wed, 22 Dec 2010 07:37:50 -0800

On 2010-12-22 14:02, Daniel Shahaf wrote:

Forwarding back to the list.

Tijn Porcelijn wrote on Wed, Dec 22, 2010 at 01:54:07 -0800:
> Hi Daniel,
>
> I guess you're right. It makes more sense to echo the external URL
> than the local directory in the "Updated external
> 'url://remote/path/to/somewhere' to revision %ld". Anyway, my
> suggestion was to make sure that "some" reference to the specific url
> was made *at all* after removing the other notice (as opposed to just
> printing a revision number and having to guess the repository it
> refers to).

I'm still not sure I understand the issue, or why you think adding that
would be useful. (Not saying that it isn't useful; just that I don't
understand why it would be.)

Before you invest more time in coding, could you please give more
concrete examples of how the current output is not satisfactory?

e.g., are you trying to parse it with a script? Or is it just that the
information you want has scrolled offscreen and you want to repeat it
nearer to the end of the output?

My personal itch is primarily that "svn up" is too verbose when using externals. This shows especially when there are no changes whatsoever.

For example, when I do update a normal directory and there are no changes (ie.: my BASE equals HEAD) I get almost no feedback: one line saying: "At revision 123". Now if I do the same for a directory that includes one svn:externals reference, I get:

 1. <<empty line>>
 2. Fetching external item into 'path/to/local/dir'
 3. External at revision 12345.
 4. <<empty line>>

For the project I'm currently working on (using 10+ externals) this easily fills up my screen.
In an (admittedly half-hearted) attempt I removed line 2 when doing svn up (but not for export and checkout). This was submitted in a different mail to this mailing list, using the same issue number.
Then, in below patch, I added the 'path/to/local/dir' to line 3, in an attempt to explain to the end-user what files the revision number is referring to. So, for every svn:externals entry I now get one line with the same output:

 1. External 'path/to/local/dir' at revision 12345.

I hope this helps explain the fix.

Tijn

Thanks,

Daniel

> I will look into printing the remote external's path.
>
> Thanks, tijn
>
> On 2010-12-22 02:54, Daniel Shahaf wrote:
>
> This has a bug, when updating a file external it displays the
> external's directory rather than the external itself. (But maybe this
> is a bug in the way the library generates the notifications?)
>
> May I ask what is the motivation for this change? The normal
> notifications (U path/to/somewhere) will always immediately precede
> the "Updated external 'path/to/somewhere' to revision %ld", so
> repeating the external's path there seems a bit redundant.
>
> I haven't run 'make check'.
>
> Daniel
>
>
> Tijn Porcelijn wrote on Fri, Nov 26, 2010 at 01:56:22 -0800:
> > [[[
> > Improves interaction, issue #3653: svn update should not output svn:external
> > * subversion/svn/notify.c (notify)
> > Add <path_local> to Externals messages
> > Note: po files should also be updated
> > ]]]
> >
> >
> >
> >
> > Hi,
> >
> > Here's a small patch for making svn:externals messages a bit more informative. With the "Fetching external item into '<path_local>'" -message removed, interpretation of svn_wc_notify_update_completed messages becomes a bit less obvious. You'll see stuff like:
> > External at revision 20
> > External at revision 2321
> > External at revision 1082367
> > At revision 19
> > The patch improves this to read:
> > External 'third-party' at revision 20
> > External 'snapshots' at revision 2321
> > External 'legacy' at revision 1082367
> > At revision 19
> > See attached notify.c.patch, Thanks,
> >
> > tijn
> >
>
> Content-Description: notify.c.patch
> > Index: subversion/svn/notify.c
> > ===================================================================
> > --- subversion/svn/notify.c (revision 1038983)
> > +++ subversion/svn/notify.c (working copy)
> > @@ -567,44 +567,66 @@
> > {
> > if (nb->is_export)
> > {
> > - if ((err = svn_cmdline_printf
> > - (pool, nb->in_external
> > - ? _("Exported external at revision %ld.\n")
> > - : _("Exported revision %ld.\n"),
> > - n->revision)))
> > - goto print_error;
> > + if (nb->in_external)
> > + err = svn_cmdline_printf
> > + (pool,
> > + _("Exported external '%s' at revision %ld.\n"),
> > + path_local,
> > + n->revision);
> > + else
> > + err = svn_cmdline_printf
> > + (pool,
> > + _("Exported revision %ld.\n"),
> > + n->revision);
> > }
> > else if (nb->is_checkout)
> > {
> > - if ((err = svn_cmdline_printf
> > - (pool, nb->in_external
> > - ? _("Checked out external at revision %ld.\n")
> > - : _("Checked out revision %ld.\n"),
> > - n->revision)))
> > - goto print_error;
> > + if (nb->in_external)
> > + err = svn_cmdline_printf
> > + (pool,
> > + _("Checked out external '%s' at revision %ld.\n"),
> > + path_local,
> > + n->revision);
> > + else
> > + err = svn_cmdline_printf
> > + (pool,
> > + _("Checked out revision %ld.\n"),
> > + n->revision);
> > }
> > else
> > {
> > if (nb->received_some_change)
> > {
> > nb->received_some_change = FALSE;
> > - if ((err = svn_cmdline_printf
> > - (pool, nb->in_external
> > - ? _("Updated external to revision %ld.\n")
> > - : _("Updated to revision %ld.\n"),
> > - n->revision)))
> > - goto print_error;
> > + if (nb->in_external)
> > + err = svn_cmdline_printf
> > + (pool,
> > + _("Updated external '%s' to revision %ld.\n"),
> > + path_local,
> > + n->revision);
> > + else
> > + err = svn_cmdline_printf
> > + (pool,
> > + _("Updated to revision %ld.\n"),
> > + n->revision);
> > }
> > else
> > {
> > - if ((err = svn_cmdline_printf
> > - (pool, nb->in_external
> > - ? _("External at revision %ld.\n")
> > - : _("At revision %ld.\n"),
> > - n->revision)))
> > - goto print_error;
> > + if (nb->in_external)
> > + err = svn_cmdline_printf
> > + (pool,
> > + _("External '%s' at revision %ld.\n"),
> > + path_local,
> > + n->revision);
> > + else
> > + err = svn_cmdline_printf
> > + (pool,
> > + _("At revision %ld.\n"),
> > + n->revision);
> > }
> > }
> > + if (err)
> > + goto print_error;
> > }
> > else /* no revision */
> > {
>
>
>
Received on 2010-12-22 16:39:57 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.