Uploaded image for project: 'Subversion'
  1. Subversion
  2. SVN-2959

[sparse-directories] depth upgrade against old servers is broken

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • trunk
    • 1.5.0
    • libsvn_ra
    • None

    Description

      This bug was revealed by the resolution of issue #2842.
      
         $ svn --version | grep " version"
         svn, version 1.5.0 (dev build)
         $ svn co --depth=empty http://svn.collab.net/repos/svn/trunk/notes/ wc
         Checked out revision 27031.
         ### note that that's a 1.4.x server, *not* a 1.5 server ###
         $ cd wc
         $ ls
         ### no files or subdirs, because depth=empty ###
         $ grep empty .svn/entries
         empty
         ### working copy also knows it's at depth=empty ###
         $ svn up
         At revision 27031.
         $ grep empty .svn/entries
         empty
         ### working copy is still at depth=empty, good ###
         $ svn up --depth=infinity
         At revision 27031.
         $ ls
         ### still no files or subdirs... this is bad... ###
         $ grep empty .svn/entries
         $ 
         ### working copy knows it is at depth=infinity now, but it doesn't ###
         ### have the items it should have for that depth.                  ###
      
      What's happening here is that the 1.4 server doesn't know about depths, so the
      1.5 client had no way to tell the server that even though the top-level
      directory claims to be at r27031, it doesn't have all/any of the entries it
      should have for that revision.  Thus when the client updated, it reported an
      r27031 directory, and the server said "Okay, you're done!".
      
      This IRC transcript outlines the solution I'm pursuing:
      
      <kfogel>     Issue #2842 (and to some degree #695) are now down to
                   this: teaching the update_editor to ignore stuff the
                   server sends back that the ambient wc depth says to
                   ignore (like, local depth is empty, and you're at an old
                   rev of the tree, and you do 'svn up' against a pre-1.5
                   server).
      
      <kfogel>     The behavior of that 'svn up' should be that it does
                   nothing to your tree, except updates it to HEAD rev.  No
                   new files or dirs get added or anything, because ambient
                   depth is empty.
      
      <kfogel>     Great, so far so good, and I have a patch that does this.
      
      <kfogel>     But what happens when you run 'svn up --depth=infinity'
                   next?
      
      <kfogel>     If the server were 1.5 or after, it would be able to
                   understand the client saying "I'm at rev
                   SOMETHING_NEAR_HEAD, but I have ambient depth of empty,
                   so I need you to send me a lot of things that haven't
                   actually changed between my rev and your new HEAD."
      
      <kfogel>     But if the server is old, it ignores the client telling
                   it about that.
      
      <kfogel>     So it just says "Oh, you're at HEAD?" (say you are at
                   head)  "Good, then I send you nothing."
      
      * kfogel ponders.
      
      <kfogel>     So, the client has to find a way to report to a pre-1.5
                   server that it's missing a lot of stuff, due to ambient
                   depths.
      
      <kfogel>     What can a 1.4 or older server understand?  It can
                   understand "nonrecursive" (but only as a requested
                   operational depth?  Or also as an ambient depth?)
      
      <kfogel>     And it can understand "r0"
      
      <sussman>    heh
      
      <sussman>    suckage
      
      <kfogel>     yep
      
      <sussman>    an old server understands things as 'missing'
      
      <sussman>    i.e. it expects a reporter-drive with a bunch of
                   'missing' things
      
      <kfogel>     sussman: but I might not know what things I'm missing
      
      <sussman>    !
      
      <sussman>    oh crap
      
      <epg>        who says new features must work with old servers?
      
      <kfogel>     If I'm at depth=empty, I don't have entries for all that
                   stuff
      
      <sussman>    well, you can say you're incomplete
      
      <kfogel>     epg: well, for your purposes, none :-)
      
      <sussman>    and thus the server sends everything
      
      <kfogel>     epg: but for the rest of the world, it would be nice if
                   all of depth worked with older servers
      
      <epg>        sure, we can't break it when you're not using the new
                   features, but once you start using --depth, you need a
                   1.5 server; that's the way merge tracking is
      
      <kfogel>     sussman: hey, that's a good idea
      
      <epg>        kfogel: meh
      
      <sussman>    kfogel: it's no different than hitting ctrl-c during a
                   checkout
      
      <epg>        kfogel: backwards-compat is nice, but no more than that
                   (and that's not google talking)
      
      <kfogel>     epg: the idea is that depthy operations might take longer
                   (you wait while the server sends you things you don't
                   need), but should work correctly.
      
      <sussman>    when you restart the checkout/update, client says, "I
                   have this directory, but it's incomplete.  here's what I
                   *do* have"
      
      <sussman>    the server then sends all the missing stuff
      
      <sussman>    IIRC, the way the client does this is by creating a
                   server-side txn (via reporter) with a 'start empty' flag
                   as TRUE
      
      <kfogel>     epg: uh, yeah, I agree, but we've been aiming for full
                   back-compat here for a while, and I don't want to stop
                   90% of the way there unless we have to.
      
      <sussman>    and then the client adds only what it knows about.  it's
                   a 'low confidence' report, is what we used to call it
      
      <kfogel>     sussman: I have to see if I can do this based on ambient
                   depth, that is, based on entry->depth
      
      <epg>        i'm not sure how useful the feature is if the server is
                   sending you crap you don't want anyway; all it saves you
                   is local storage, not time
      
      <epg>        but if you're 90% there, sure, keep going :)
      
      <kfogel>     Which ought to be possible, since we clearly have the
                   entry available when we send the start_empty flag.
      
      <kfogel>     epg: not just local storage, but local clutter (I don't
                   want 100 directories mucking up my working copy, e.g.)
      
      <vgeorgescu> kfogel: you can use the start_empty flag, but you need a
                   way to distinguish between old servers, which need it,
                   and new servers, which don't
      
      <vgeorgescu> and you need to make this distinction in libsvn_wc
      
      <kfogel>     vgeorgescu: I'm in adm_crawler.c right now, yeah
      
      <kfogel>     vgeorgescu: there is no way to distinguish between old an
                   new servers reliably across all RA layers, as far as I
                   know.  But, I'm not sure that sending start_empty=TRUE to
                   new servers will hurt, depending on depth.
      
      <kfogel>     For example, I think in local-depth=empty case, it might
                   be okay.
      
      <kfogel>     I'm describing my current mental state to you, as it
                   evolves.  Any thoughts welcome!
      
      <kfogel>     Not sure yet about depth-files and depth-immediates
      
      <vgeorgescu> yeah, but new servers can handle depth upgrades without
                   start empty, so it's more efficient
      
      <kfogel>     ah, right
      
      <kfogel>     dang it, dang it
      
      <kfogel>     If you can think of a way to know (before we start the
                   reporter crawl) whether the server is depth-aware, I can
                   easily add a flag to svn_wc_crawl_revisions3() (or 4(), i
                   mean)
      
      <kfogel>     In the DAV cases, I think we know, because a feature
                   exchange goes on.  And in ra_local, we know.
      
      <kfogel>     But ra_svn?
      
      <kfogel>     Oh, wait: we have some control over that, because we
                   haven't released 1.5 yet.
      
      <kfogel>     Hmmmm!
      
      <vgeorgescu> I was thinking of adding a server_supports_depth()
                   function to the reporter
      
      <kfogel>     vgeorgescu: yes
      
      <kfogel>     one sec, let me look at something
      
      <vgeorgescu> and each ra_layer can implement it
      
      <kfogel>     Okay, in libsvn_ra_serf/update.c:start_report(), we learn
                   whether the server is depth-aware before we start the
                   editor drive
      
      <vgeorgescu> that's too late
      
      <kfogel>     and in libsvn_ra_neon/fetch.c:start_element()
      
      <kfogel>     yes
      
      <kfogel>     too late
      
      <kfogel>     I was just thinking that :-).
      
      <vgeorgescu> ra_svn is easy, just add a new capability
      
      <kfogel>     So, do all RA layers currently have a feature-exchange
                   mechanism that is completed before we begin the report?
      
      <vgeorgescu> not sure about ra_neon/ra_serf
      
      <kfogel>     I'll take a look.
      
      <kfogel>     Thanks for the excellent ideas.
      
      <vgeorgescu> happy to help
      
      <kfogel>     vgeorgescu: in ra_svn, I see a way for the client to tell
                   the server about the client's capabilities, but does the
                   server tell the client about the server's capabilities?
      
      <kfogel>     vgeorgescu: never mind, got it
      
      <kfogel>     glasser: Is there a capabilities negotition in the DAV
                   protocol already?  I need the client to know whether or
                   not the server supports 'depth', preferably before the
                   client starts its update report.
      
      <sussman>    yes, the OPTIONS request is for that
      
      <glasser>    see "dav makes everything arbitrarily complicated".  i
                   believe the answer is "sort of"
      
      <glasser>    as in "there's this thing we could use for negotiation,
                   but we don't"
      
      <kfogel>     You mean we don't currently send any capabilities?
      
      <kfogel>     (in DAV)?
      
      <glasser>    I'm not really the right person to ask
      
      <glasser>    My knowledge of this mostly consists of being in the same
                   situation as you and being sad
      
      <glasser>    But my impression of DAV is that every single type of
                   request is different and quirky, instead of having one
                   standard handshake like ra_svn
      
      <kfogel>     oh fugg
      
      <kfogel>     ok, thanks
      
      <sussman>    yep
      
      <sussman>    in theory, OPTIONS is the equivalent of the ra_svn
                   feature-negotiation
      
      <sussman>    we just don't use it consistently
      
      <glasser>    and adding OPTIONS requires a complete request-response
                   turnaround, which might be a lot of overhead?  i'm not
                   clear on the details
      
      <kfogel>     sussman: see backscroll -- the client needs to know if
                   the server supports depth *before* the client starts an
                   update report.  Will it require adding a network
                   turnaround to get this information to the client?
      
      <sussman>    hm, I suspect so
      
      <sussman>    the server should advertise a 'depth' feature in its
                   OPTIONS response
      
      <epg>        i think we alerady send an OPTIONS request at the start
                   of every operation
      
      <sussman>    if not present, the client should assume it's a pre-1.5 server
      
      <epg>        "operation" == svn log, not individual http requests
      
      <sussman>    right
      
      <sussman>    each RA session kicks off with an OPTIONS
      
      <glasser>    oh, huh, i thought it was inconsistent. if not, awesome
      
      <epg>        i can't say with any degree of certainty; it's just
                   something i've noticed back when i used to look at apache
                   log files a lot
      
      <kfogel>     sussman: great!  Then no extra turnaround required.
      
      <dlr_>       kfogel: Regarding OPTIONS, I didn't use it for the new
                   mergeinfo REPORT -- I just try the report instead.
      
      <kfogel>     dlr_: this is a little different, I think.  We need to
                   tell the server what to send down, before it sends stuff.
      
      <kfogel>     dlr_: that is, we need to know whether or not to sent
                   start_empty=true, which we should send only to old
                   servers when depth is not infinity, in order to get it to
                   send entries it wouldn't otherwise send.
      
      <dlr_>       Sounds like an OPTIONS request makes a lot of sense for
                   this case.
      

      Attachments

        1. 4_2959-patch-in-progress-4.txt
          40 kB
          Karl Fogel
        2. 3_2959-patch-in-progress-3.txt
          39 kB
          Karl Fogel
        3. 2_2959-patch-in-progress-2.txt
          25 kB
          Karl Fogel
        4. 1_2959-patch-in-progress-1.txt
          18 kB
          Karl Fogel

        Issue Links

          Activity

            People

              Unassigned Unassigned
              kfogel Karl Fogel
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: