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

Sparse directories - mini-review

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2007-03-15 18:41:24 CET

Hi Karl,

Since you mentioned that you were planning to merge the
sparse-directories branch to trunk shortly, I thought I'd take a quick
look at the branch diff. I've not reviewed it thoroughly, but here's
some random comments:

Meta-comments:

- There's an awful lot of TODOs, but I get the sense that they're intended
  to be exhaustive rather than indicate where you think there's a problem
  that absolutely needs fixing.

- Is the intention to get sparse directories to work completely for 1.5,
  or just to get it to work at least as well as -N currently?

- If the latter, the only blocker for release seems to be the work needed
  to make a client with a sparse wc ignore data sent by sparse-ignorant
  servers. What exactly is the status of this? Is this something that's
  absolutely necessary for 1.5?

ra_svn:

- ra_svn: In the 'update', 'switch', 'status', and 'diff' commands,
  we're adding the depth inside a tuple, whereas in the 'set-path' and
  'link-path' cases we're not. The former matches what the extensibility
  parts of the protocol document suggest, but the latter makes more sense
  to me (I don't understand what the protocol document is saying here).
  Regardless, can we pick one approach?

- libsvn_ra_svn/protocol is missing the changes to all commands apart from
  set-path and link-path.

- svnserve/serve.c:diff() - You've added an optional depth tuple inside
  code that checks for a 1.4 request (one without text_deltas).
  You should set depth_word to NULL directly rather than rely on
  svn_ra_svn_parse_tuple to do it for you - it's just confusing as-is.

Other:

- (This isn't exclusive to sparse-directories: changelist and keep-local
  do it too, but:) Why are we writing out the depth attribute in the
  old XML-format entries, when by definition it can never have any value
  other than the default? (Also note that the specification for the
  entries XML format prohibits unknown attributes, so this is
  technically wrong).

  (Actually, could someone remind me why we even have the capability to
  write out the XML entries format? I'm sure there is a reason, but I
  can't remember what it is.)

- The help for the command-line option --depth doesn't mention that
  'infinity' is a valid option (though it is referenced in the help for
  --recursive).

- If we're updating the XML output for 'svn info', we should update the
  schema too (in subversion/schema/info.rnc).

- svn_types.h: Contrary to the comments, you use svn_depth_unknown all
  over the place. However, you don't use svn_depth_exclude -- are you
  fairly sure you'll need it, or would it be better to get rid of it
  until a need comes up?

- libsvn_wc/entries.h has a reference to non-existent svn_depth_ignore.

- Looking at the branch diff for include/svn_ra.h, I notice you deleted
  a lot of comments of the form "the RA layer assumes that all paths are
  relative to the URL used to open @a session." (see r21314). Any reason?
  It seems unrelated to the other changes.

Regards,
Malcolm

  • application/pgp-signature attachment: stored
Received on Thu Mar 15 18:41:43 2007

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