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

Re: More SVN status bugs

From: Karl Fogel <kfogel_at_galois.collab.net>
Date: 2001-01-11 19:25:28 CET

"Bill Tutt" <billtut@microsoft.com> writes:
> * ${SVN_PROG} delete --force ${TEST_DIR_1}\A\D\H\omega
>
> Registers the delete in $(TEST_DIR_1)/SVN/entries with a filename of
> "A\D\H\omega" and an ancestor of: "anni/A\D\H\omega"
> Doesn't touch ANYTHING in $(TEST_DIR_1)/A/D/H/SVN/entries

Okay; working on, thanks for noticing this! (Time to get that path
library fleshed out. :-) ).

> * adding a file (newfile1), and then deleting it before you commit it
> results in the correct state in the entry structure for it, but there's
> an unneeded else that marks this file as just being added:
>
> Index: status.c
> ===================================================================
> RCS file: /cvs/subversion/subversion/libsvn_wc/status.c,v
> retrieving revision 1.24
> diff -u -r1.24 status.c
> --- status.c 2001/01/09 00:05:22 1.24
> +++ status.c 2001/01/11 08:52:54
> @@ -108,7 +108,7 @@
> status->prop_status = svn_wc_status_added;
> }
>
> - else if (entry->state & SVN_WC_ENTRY_DELETED)
> + if (entry->state & SVN_WC_ENTRY_DELETED)
> {
> status->text_status = svn_wc_status_deleted;

Hmm. If we get rid of the `else', then the result of the second
conditional body might simply override the result of the first. In
other words, something that was marked as *both* added and deleted
would only show up as deleted. Which would be fairly random -- if the
conditionals' order were reversed, then it would be adds overriding
deletes instead of the other way around.

I think the `else' is deliberate. Here's the reasoning:

   1. If an entry is just added, you want to report it as added.

   2. If an entry is just deleted, you want to report it as deleted.

   3. If an entry is _both_ added and deleted, that can only mean one
      thing: the entry was deleted and *then* added. That is, in the
      working copy, someone removed a file, then they added a new file
      under the same name. (Had the order been reversed, then the
      entry would simply have been removed -- there's no point even
      talking about a file that was added and then un-added. It never
      happened.) So the idea is that status reports it as added.

The current conditional setup supports this logic. (But maybe there
should be a comment to this effect nearby!)

Another solution is to have a special status for "replaced", that is,
deleted and then added. Thoughts?

> * svn_wc_text_modified_p was assuming that just because we could
> construct a string variable that might point to the text-base version of
> the file, that the actual file exists.

Nice catch. :-) Yeah, your patch for this looks like exactly the right
thing, go ahead and commit.

Note that that whole block of code (handling the case where the
text-base file doesn't exist) was written in anticipation of the day
when working copies don't require a text base. That day is not today,
nor is it the near future. However, when and if that day arrives,
this function will be prepared for it. :-)

-K

> Index: questions.c
> ===================================================================
> RCS file: /cvs/subversion/subversion/libsvn_wc/questions.c,v
> retrieving revision 1.44
> diff -u -r1.44 questions.c
> --- questions.c 2001/01/08 23:06:34 1.44
> +++ questions.c 2001/01/11 08:53:21
> @@ -349,9 +349,12 @@
> /* Get the full path of the textbase revision of filename */
> textbase_filename = svn_wc__text_base_path (filename, 0, pool);
>
> + err = svn_io_check_path (textbase_filename, &kind, pool);
> + if (err) return err;
> +
> /* Simple case: if there's no text-base revision of the file, all we
> can do is look at timestamps. */
> - if (! textbase_filename)
> + if (kind != svn_node_file)
> {
> err = timestamps_equal_p (&equal_timestamps, filename,
> svn_wc__text_time, pool);
Received on Sat Oct 21 14:36:19 2006

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.