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

Re: Should we always report_revisions?

From: Ben Collins-Sussman <sussman_at_collab.net>
Date: 2001-09-17 00:54:17 CEST

Mo, this is the very issue Kevin has submitted multiple patches for.
It has to do with the way we're dealing with "anchors/targets", about
which Mike Pilato sent out a huge mail. This bug will be fixed RSN;
I'm going to mark your email here as part of the larger discussion of
this problem -- and how some redesign may be needed. :-)

Mo DeJong <supermo@bayarea.net> writes:

> Hi all.
>
> I have been doing some hacking around in adm_crawler.c in an attempt to
> learn how to fix a problem with the update command. Thing is, I am
> running into some problems and I was wondering if someone more
> familiar with the code could give me some pointers.
>
> Here is the problem I am trying to fix:
>
> % cd svn/trunk
> % rm README
> % svn up
> A ./README
>
> That works as expected. but this does not!
>
> % rm README
> % svn up README
>
> Bummer eh?
>
> I did some looking around in the code and the problem seems to be rooted in
> svn_wc_crawl_revisions from adm_crawler.c. There is an if that basically does
> this:
>
> if (dir)
> report_revisions()
> else
> ...
>
> In this directory case (dir is "." for `svn update') the report_revisions() function
> does a bunch of stuff like check the .svnignore file, check to make sure the
> file actually exists on disk, and so on. If the file is missing on disk, the
> reporter->delete_path() method is called to add a <S:missing>README</S:missing>
> tag to the file sent over to the DAV layer. That will pull down a new copy of
> README (this could be optimized in the case where the rev is the same as
> the copy in SVN/text-base/README, ala svn revert).
>
> Trouble is, I can't seem to figure out how to implement this sort of thing for
> the case where the file name is given on the command line. I tried to check
> for the file on disk and add a missing tag to the info sent over to the DAV
> layer, but that seems to blow up on the server side since the URL is for
> a file and not a directory.
>
> Like so:
>
>
> Index: subversion/libsvn_wc/adm_crawler.c
> ===================================================================
> --- subversion/libsvn_wc/SVN/text-base/adm_crawler.c Mon Sep 10 17:16:47 2001
> +++ subversion/libsvn_wc/adm_crawler.c Sun Sep 16 12:28:38 2001
> @@ -1797,27 +1797,58 @@
> return err;
> }
> }
> - else if ((entry->kind == svn_node_file)
> - && (entry->revision != base_rev))
> + else if (entry->kind == svn_node_file)
> {
> - /* If this entry is a file node, we just want to report that
> - node's revision. Since we are looking at the actual target
> - of the report (not some file in a subdirectory of a target
> - directory), and that target is a file, we need to pass an
> - empty string to set_path. */
> - err = reporter->set_path (report_baton,
> - svn_stringbuf_create ("", pool),
> - base_rev);
> - if (err)
> + apr_status_t apr_err;
> + apr_file_t *stathandle = NULL;
> +
> + /* If this entry is a file node and the revision differs from
> + the most recent one in the WC, then report the revision number.
> + Since we are looking at the actual target of the report
> + (not some file in a subdirectory of a target directory),
> + and that target is a file, we need to pass an empty string
> + to set_path. */
> +
> + if (entry->revision != base_rev)
> {
> - /* Clean up the fs transaction. */
> - svn_error_t *fserr;
> - fserr = reporter->abort_report (report_baton);
> - if (fserr)
> - return svn_error_quick_wrap (fserr, "Error aborting report.");
> - else
> - return err;
> + err = reporter->set_path (report_baton,
> + svn_stringbuf_create ("", pool),
> + base_rev);
> +
> + if (err)
> + {
> + /* Clean up the fs transaction. */
> + svn_error_t *fserr;
> + fserr = reporter->abort_report (report_baton);
> + if (fserr)
> + return svn_error_quick_wrap (fserr, "Error aborting report.");
> + else
> + return err;
> + }
> }
> +
> + /* If the named file does not exist in the WC, report it as missing. */
> +
> + apr_err = apr_file_open (&stathandle, path->data, APR_READ,
> + APR_OS_DEFAULT, pool);
> +
> + if (apr_err)
> + {
> + err = reporter->delete_path (report_baton, path);
> +
> + if (err)
> + {
> + /* Clean up the fs transaction. */
> + svn_error_t *fserr;
> + fserr = reporter->abort_report (report_baton);
> + if (fserr)
> + return svn_error_quick_wrap (fserr, "Error aborting report.");
> + else
> + return err;
> + }
> + }
> + else
> + apr_file_close (stathandle);
> }
>
> /* Finish the report, which causes the update editor to be driven. */
>
>
>
> Since this approach does not seem to be going anywhere, I was thinking
> that perhaps it would be better to roll the update with file names case
> into the report_revisions function. There is already code in there to
> filter out file names that appear in the .svnignore file. Does it make
> sense to pass in an array of file names and filter out files that do not
> appear in the array? That might be a little tricky since we would also
> need to pass an option to turn recursion on or off. I was thinking we
> might need that anyway because we would want an option like
> --local to turn off recursion for subcommands like update.
>
> Any advice?
> Mo DeJong
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:41 2006

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