[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: Thu, 23 Dec 2010 00:09:32 -0800

Hi Daniel,

Here's my previous mail as plain text (utf8).

tijn

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-23 09:11:38 CET

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