C. Michael Pilato wrote:
> > r1039140 | julianfoad | 2010-11-25 13:51:34 -0500 (Thu, 25 Nov 2010) | 4 lines
> > Changed paths:
> > M /subversion/trunk/subversion/svn/update-cmd.c
> >
> > Fix assertion failures.
> >
> > * subversion/svn/update-cmd.c
> > (print_update_summary): Don't allow a URL to reach svn_dirent_*().
>
> Why?
I should have explained in the log message. I answered Mike on IRC, but
for the record:
The main issue is that update *shouldn't* be operating with a URL as a
target: it doesn't make sense - but the client impl doesn't reject URL
targets from being passed through.
So sometimes - even though the output is "Skip" - a URL does reach here.
$ svn up http://...
Skipped 'http://...'
Summary of updates:
*bang*
> First, you've prevented the call to svn_dirent_get_absolute, yes. But
> immediately below that are two more calls to svn_dirent_* APIs which are not
> similarly avoided. I'll copy the code context here for your convenience:
>
> /* Convert to an absolute path if it's not already. */
> /* (It shouldn't be URL, but don't call svn_dirent_* if it is.) */
> if (! svn_path_is_url(path) && ! svn_dirent_is_absolute(path))
> SVN_ERR(svn_dirent_get_absolute(&path, path, iter_pool));
>
> /* Remove the current working directory prefix from PATH (if
> PATH is at or under $CWD), and convert to local style for
> display. */
> path_local = svn_dirent_skip_ancestor(path_prefix, path);
> path_local = svn_dirent_local_style(path_local, iter_pool);
Oops. I think I just missed that in my haste to check in a fix for the
buildbot failures. You're right, that's poor.
> But that's not my biggest concern. If, as you say, the path "shouldn't be
> URL", then why is it a problem that an assertion verifies as much? I'm
> really down on spineless code of this sort.
>
> Please revert this change or provide a reasonable explanation for it.
We agreed that C-Mike will clean up print_update_summary() and I will
make the caller reject URL targets in the first place, like Noorul has
done for other subcommands recently.
- Julian
Received on 2010-11-29 18:38:23 CET