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

clean up svn_wc_repos_add_repos_file() situation

    XMLWordPrintableJSON

    Details

    • Type: Task
    • Status: Closed
    • Priority: Major
    • Resolution: Cannot Reproduce
    • Affects Version/s: trunk
    • Fix Version/s: 1.8-consider
    • Component/s: libsvn_wc
    • Labels:

      Description

      This is a code-cleanup task broken out of issue #1552.  The text below is copied
      directly from that issue, but note that Ramaswamy's patch was eventually
      applied, so although the need for a cleanup remains, some minor bits in the
      remarks below are now obsolete.  The main points still stand.
      
                                 =*= =*= =*= =*= =*=
      
      Okay, talked with Ramaswamy a bit.  There are several semi-related things going
      on here, let's try to untangle it all:
      
      * His patch above could be used for the 1.0.x series.
      
      * But most likely we want to fix this in trunk (it may never even get backported
      to 1.0.x).  The cleanest way to do that is to add svn_wc_add_repos_file2(),
      which would be just like its namesake, except that it would take an (optionally
      null) notify_func and baton.  The old svn_wc_add_repos_file() would just wrap
      the new function, passing null for the notify_func/baton.  Then in a separate
      commit, we deprecate svn_wc_add_repos_file() and change all callers over to the
      new function.
      
      * However, I seem to remember long-standing agreement that we all hate the name
      svn_wc_add_repos_file() and wish we had called it something else.  The "foo2"
      convention is just for convenience; we are not obligated to name the new
      function svn_wc_add_repos_file2().  We can call it anything we want, as long as
      the old, deprecated function refers to the new one.  How about
      svn_wc_add_file_from_history() or svn_wc_add_file_with_history()?  But see the
      next point...
      
      * In general, the relationship between svn_wc_copy(), svn_wc_add(), and
      svn_wc_add_repos_file() is ill-thought-out.  The doc string for
      svn_wc_add_repos_file() alludes to this a bit.  While svn_wc_add_repos_file() is
      certainly a terrible name, the next most natural choices are things like
      svn_wc_add_file_with_history(), which of course makes one wonder "How is this
      different from svn_wc_copy()?"  Don't have a solution to offer here, just
      pointing out that cleanly resolving this issue may involve tacking a larger mess.
      
      * Independently of ALL of the above, why does notify.c:notify() ignore
      'svn_wc_notify_copy'?  Both Ramaswamy's patch above and a new, notifying version
      of svn_wc_add_repos_file() should presumably send that code for the
      notification.  In fact, I think he tried that first, and then fell back to
      'svn_wc_notify_update_add' when notify_copy didn't produce any output.
      

        Attachments

          Issue Links

            Activity

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: