Re: svn commit: rev 4347 - in trunk/subversion: libsvn_client tests/clients/cmdline tests/clients/cmdline/svntest

On Mon, Jan 13, 2003 at 02:55:43PM -0600, Karl Fogel wrote:
> Josef Wolf <jw@raven.inka.de> writes:

> > Hmmm, wouldn't it be better to make more sanity checks in
> > svn_wc_entry? This would catch not only "svn add" but some other
> > sub-commands, too.
> >
> > Just curious...

> I honestly don't know. Want to add the code and run 'make check', see
> what happens? (I'm not sure exactly what change you're proposing.)

I had something like this in mind:

Index: subversion/libsvn_wc/entries.c
===================================================================
--- subversion/libsvn_wc/entries.c (revision 4388)
+++ subversion/libsvn_wc/entries.c (working copy)
@@ -649,6 +649,10 @@
         }
     }

+ if (kind == svn_node_unknown)
+ return svn_error_createf
+ (SVN_ERR_NODE_UNKNOWN_KIND, NULL, "Unknown kind for \"%s\"", path);
+
   if (kind != svn_node_dir || ! is_wc)
     {
       /* Maybe we're here because PATH is an unversioned directory, in

This would fix not only the edge-case you fixed in r4347. In addition,
this would catch _all_ attempts to fiddle around with unknown file types
(e.g. symlinks) in the entries file. In addition to the edge-case you
fixed in r4347, it would for example catch the following:

  # AFAIK this would be a new issue not yet filed in the issue-DB
  svnadmin create test
  svn co file://`pwd`/test wc
  cd wc
  touch x
  svn add x
  rm x
  ln -s y x
  svn stat
  # "svn stat" gives _no_ output at all!

Please note that the above patch is _not_ exactly what I am
proposing. It is just to show the idea: make proper checks on
strategically good places instead of fiddling around with numerous
edge-cases. Go the "generic" way instead of countless special
cases. IMHO svn_wc_entry() is one of those strategically good places
where the more "generic" check can be done because it is called by
every subcommand that needs to do something in the WC.

The diff attached above is OTOH somewhat too harsh to "svn stat".
Instead bailing out, it should output "~ x".

BTW: I wonder why test 6 of stat_tests.py explicitly requires "svn stat" to
     silently ignore the replacement of the versioned item by a symlink?

BTW1: The failure of stat_tests.py after the above patch was actually
      the reason why I found this "svn stat" edge-case.

BTW2: I think that going the "generic" way would be a benefit.
      Experience shows that edge-cases are found mostly by
      accident. Only heaven knows how many are still waiting for
      us. Fixing them one-by-one will result in a looong stony way.

-- 
-- Josef Wolf -- jw@raven.inka.de --
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org