You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David Kimdon <dw...@debian.org> on 2003/06/02 02:17:21 UTC

[rfc/patch] : #933,#735 import syntax, mkdir -p

Hi,

Here is a patch for the import issues indicated below.  Any comments
and/or criticism is, of course, welcome.  I'm particularly interested
in comments on the change I made to get_ra_editor().  I need something
like 'stat()' for the repository, and I believe that this extra error
checking in get_ra_editor() does no harm, but maybe I missed
something.  The change to subversion/libsvn_client/commit.c(import) is
similar to that which has been reviewed by cmpilato previously.

-David

Fix issue #933: svn import syntax is confusing; merge 1st and 3rd arguments
Fix issue #735: import should 'mkdir -p'

Import syntax is now:

svn import [PATH] URL

Parent directories in URL are created as necessary.  Testsuite and
documentation updated as well.

* subversion/include/svn_client.h (svn_client_import) : Remove 'new_entry'
  argument.  Trailing entries in URL that do not exist in the repository
  are considered to be a new path to create.
* subversion/libsvn_client/commit.c
  (import) :  Interpret 'new_entry' as a path to be created in the repository
  within which the actual import will take place.
  (get_ra_editor) : Return an error if 'base_url' does not correspond to a 
  repository path.
  (svn_client_import) : If the first attempt to open a repository session
  fails due to a non-existent URL, back up a directory and try again.  This
  allows us to create 'new_entry' dynamically rather than requiring an 
  argument.
* subversion/clients/cmdline/import-cmd.c : 'import' now takes at most
  two rather than three arguments.  PATH now comes before URL.
* subversion/tests/clients/cmdline/basic_tests.py : Update single file
  import test so parent directories are created to hold the actual
  import.

Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h	(revision 6108)
+++ subversion/include/svn_client.h	(working copy)
@@ -573,24 +573,18 @@
  * head, authenticating with the authentication baton cached in @a ctx, 
  * and using @a ctx->log_msg_func/@a ctx->log_msg_baton to get a log message 
  * for the (implied) commit.  Set @a *commit_info to the results of the 
- * commit, allocated in @a pool.
+ * commit, allocated in @a pool.  If some components of @a url do not exist
+ * then create parent directories as necessary.
  *
- * @a new_entry is the new entry created in the repository directory
- * identified by @a url.  @a new_entry may be null (see below), but may 
- * not be the empty string.
- *
  * If @a path is a directory, the contents of that directory are
- * imported, under a new directory named @a new_entry under @a url; or 
- * if @a new_entry is null, then the contents of @a path are imported 
- * directly into the directory identified by @a url.  Note that the 
+ * imported directly into the directory identified by @a url.  Note that the
  * directory @a path itself is not imported -- that is, the basename of 
  * @a path is not part of the import.
  *
- * If @a path is a file, that file is imported as @a new_entry (which may
- * not be @c NULL).
+ * If @a path is a file, then the dirname of @a url is the directory
+ * receiving the import.  The basename of @a url is the filename in the
+ * repository.  In this case if @a url already exists, return error.
  *
- * In all cases, if @a new_entry already exists in @a url, return error.
- * 
  * If @a ctx->notify_func is non-null, then call @a ctx->notify_func with 
  * @a ctx->notify_baton as the import progresses, with any of the following 
  * actions: @c svn_wc_notify_commit_added,
@@ -614,7 +608,6 @@
 svn_error_t *svn_client_import (svn_client_commit_info_t **commit_info,
                                 const char *path,
                                 const char *url,
-                                const char *new_entry,
                                 svn_boolean_t nonrecursive,
                                 svn_client_ctx_t *ctx,
                                 apr_pool_t *pool);
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c	(revision 6108)
+++ subversion/libsvn_client/commit.c	(working copy)
@@ -310,8 +310,9 @@
  * directory, NEW_ENTRY may be null, which creates as many new entries
  * in the top repository target directory as there are entries in the
  * top of PATH; but if NEW_ENTRY is non-null, it is the name of a new
- * subdirectory in the repository to hold the import.  If PATH is a
- * file, NEW_ENTRY may not be null.
+ * subdirectory in the repository to hold the import.  NEW_ENTRY can have
+ * multiple components in which case parent directories are created as
+ * needed. If PATH is a file, NEW_ENTRY may not be null.
  * 
  * NEW_ENTRY can never be the empty string.
  * 
@@ -341,6 +342,7 @@
   void *root_baton;
   svn_node_kind_t kind;
   apr_array_header_t *ignores;
+  apr_array_header_t *batons = NULL;
 
   /* Get a root dir baton.  We pass an invalid revnum to open_root
      to mean "base this on the youngest revision".  Should we have an
@@ -351,6 +353,40 @@
   /* Import a file or a directory tree. */
   SVN_ERR (svn_io_check_path (path, &kind, pool));
 
+  if (new_entry)
+    {
+      apr_array_header_t *dirs;
+      const char *new_path = "";
+      int i;
+
+      dirs = svn_path_decompose (new_entry, pool);
+
+      /* If we are importing a file then NEW_ENTRY's basename is
+       * the desired filename in the repository.  We don't create
+       * a directory with that name. */
+      if (kind == svn_node_file)
+        apr_array_pop (dirs);
+
+      for (i = 0; i < dirs->nelts; i++)
+        {
+          void *temp;
+
+          if (!batons)
+            batons = apr_array_make (pool, 1, sizeof (void *));
+
+          *((void **) apr_array_push (batons)) = root_baton;
+
+          new_path = svn_path_join (new_path, ((char **)dirs->elts)[i],
+                                    pool);
+
+          SVN_ERR (editor->add_directory (new_path,
+                                          root_baton,
+                                          NULL, SVN_INVALID_REVNUM,
+                                          pool, &temp));
+          root_baton = temp;
+        }
+    }
+
   /* Note that there is no need to check whether PATH's basename is
      the same name that we reserve for our admistritave
      subdirectories.  It would be strange, but not illegal to import
@@ -374,14 +410,6 @@
     }
   else if (kind == svn_node_dir)
     {
-      void *new_dir_baton = NULL;
-
-      /* Grab a new baton, making two we'll have to close. */
-      if (new_entry)
-        SVN_ERR (editor->add_directory (new_entry, root_baton,
-                                        NULL, SVN_INVALID_REVNUM,
-                                        pool, &new_dir_baton));
-
 #if 0 /* Temporarily blocked out for consideration, see below. */
       /* If we activate this notification, then
        *
@@ -415,13 +443,10 @@
 #endif /* 0 */
 
       SVN_ERR (import_dir (editor, 
-                           new_dir_baton ? new_dir_baton : root_baton, 
+                           root_baton,
                            path, new_entry ? new_entry : "",
                            nonrecursive, excludes, ctx, pool));
 
-      /* Close one baton or two. */
-      if (new_dir_baton)
-        SVN_ERR (editor->close_directory (new_dir_baton, pool));
     }
   else if (kind == svn_node_none)
     {
@@ -432,6 +457,15 @@
 
   /* Close up the show; it's time to go home. */
   SVN_ERR (editor->close_directory (root_baton, pool));
+
+  if (batons)
+    {
+      void **baton;
+
+      while ((baton = (void **) apr_array_pop (batons)))
+        SVN_ERR (editor->close_directory (*baton, pool));
+    }
+
   SVN_ERR (editor->close_edit (edit_baton, pool));
 
   return SVN_NO_ERROR;
@@ -457,6 +491,8 @@
                svn_boolean_t is_commit,
                apr_pool_t *pool)
 {
+  svn_node_kind_t kind;
+
   /* Get the RA vtable that matches URL. */
   SVN_ERR (svn_ra_init_ra_libs (ra_baton, pool));
   SVN_ERR (svn_ra_get_ra_library (ra_lib, *ra_baton, 
@@ -469,6 +505,13 @@
                                         is_commit, !is_commit,
                                         ctx, pool));
 
+  SVN_ERR ((*ra_lib)->check_path (&kind, *session, "", SVN_INVALID_REVNUM, pool));
+
+  if (kind == svn_node_none)
+    return svn_error_createf (SVN_ERR_FS_NO_SUCH_ENTRY, NULL,
+                              "the path '%s' does not exist",
+                              base_url);
+
   /* Fetch the latest revision if requested. */
   if (latest_rev)
     SVN_ERR ((*ra_lib)->get_latest_revnum (*session, latest_rev, pool));
@@ -486,7 +529,6 @@
 svn_client_import (svn_client_commit_info_t **commit_info,
                    const char *path,
                    const char *url,
-                   const char *new_entry,
                    svn_boolean_t nonrecursive,
                    svn_client_ctx_t *ctx,
                    apr_pool_t *pool)
@@ -501,20 +543,9 @@
   const char *committed_date = NULL;
   const char *committed_author = NULL;
   apr_hash_t *excludes = apr_hash_make (pool);
+  const char *new_entry = NULL;
 
-  /* Sanity check: NEW_ENTRY can be null or non-empty, but it can't be
-     empty. */
-  if (new_entry && (strcmp (new_entry, "") == 0))
-    return svn_error_create (SVN_ERR_FS_PATH_SYNTAX, NULL,
-                             "empty string is an invalid entry name");
 
-  /* The repository doesn't know about the reserved. */
-  if (new_entry && strcmp (new_entry, SVN_WC_ADM_DIR_NAME) == 0)
-    return svn_error_createf
-      (SVN_ERR_CL_ADM_DIR_RESERVED, NULL,
-       "the name \"%s\" is reserved and cannot be imported",
-       SVN_WC_ADM_DIR_NAME);
-
   /* Create a new commit item and add it to the array. */
   if (ctx->log_msg_func)
     {
@@ -550,18 +581,68 @@
     {
       svn_node_kind_t kind;
       const char *base_dir = path;
+      apr_array_header_t *dirs = NULL;
+      const char *temp;
+      const char *dir;
 
       SVN_ERR (svn_io_check_path (path, &kind, pool));
       if (kind == svn_node_file)
         svn_path_split (path, &base_dir, NULL, pool);
 
-      SVN_ERR (get_ra_editor (&ra_baton, &session, &ra_lib, NULL,
-                              &editor, &edit_baton, ctx, url, base_dir,
-                              NULL, log_msg, NULL, &committed_rev,
-                              &committed_date, &committed_author, 
-                              FALSE, pool));
+      err = NULL;
+
+      do
+        {
+          if (err)
+            {
+              /* If get_ra_editor below failed we may either tried to open
+               * an invalid url, or else some other kind of error.  In case
+               * the url was bad we back up a directory and try again. */
+
+              if (err->apr_err != SVN_ERR_FS_NO_SUCH_ENTRY)
+                return err;
+
+              if (!dirs)
+                dirs = apr_array_make (pool, 1, sizeof(const char *));
+
+              svn_path_split (url, &temp, &dir, pool);
+              *((const char **)apr_array_push (dirs)) = dir;
+              url = temp;
+            }
+        }
+      while ((err = get_ra_editor (&ra_baton, &session, &ra_lib, NULL,
+                                   &editor, &edit_baton, ctx, url, base_dir,
+                                   NULL, log_msg, NULL, &committed_rev,
+                                   &committed_date, &committed_author,
+                                   FALSE, pool)));
+
+      if (dirs)
+        {
+          const char **child;
+
+          if ((child = (const char **) apr_array_pop(dirs)))
+            {
+              new_entry = *child;
+
+              while ((child = (const char **) apr_array_pop(dirs)))
+                new_entry = svn_path_join (new_entry, *child, pool);
+            }
+        }
+
+      if (kind == svn_node_file && ! new_entry)
+        return svn_error_createf
+          (SVN_ERR_ENTRY_EXISTS, NULL,
+           "the path \"%s\" already exists", url);
     }
 
+  /* The repository doesn't know about the reserved. */
+  if (new_entry && strcmp (new_entry, SVN_WC_ADM_DIR_NAME) == 0)
+    return svn_error_createf
+      (SVN_ERR_CL_ADM_DIR_RESERVED, NULL,
+       "the name \"%s\" is reserved and cannot be imported",
+       SVN_WC_ADM_DIR_NAME);
+
+
   /* If an error occured during the commit, abort the edit and return
      the error.  We don't even care if the abort itself fails.  */
   if ((err = import (path, new_entry,
Index: subversion/clients/cmdline/man/svn.1
===================================================================
--- subversion/clients/cmdline/man/svn.1	(revision 6108)
+++ subversion/clients/cmdline/man/svn.1	(working copy)
@@ -220,7 +220,7 @@
 command is entered, a short description on how to use that command is
 presented.
 .TP
-\fBimport\fP \fIRepository-URL\fP [\fIPath\fP] [\fINew-Repository-Entry\fP] [\fI--username name\fP] [\fI--password pass\fP] [\fI--no-auth-cache\fP] [\fI--encoding\fP] [\fI-FmqN\fP]
+\fBimport\fP [\fIPath\fP] \fIRepository-URL\fP [\fI--username name\fP] [\fI--password pass\fP] [\fI--no-auth-cache\fP] [\fI--encoding\fP] [\fI-FmqN\fP]
 Import a file or tree into the repository.
 .TP
 \fBinfo\fP \fItarget1\fP [\fItarget2\fP ...] [\fI--targets file\fP] [\fI-R\fP]
Index: subversion/clients/cmdline/import-cmd.c
===================================================================
--- subversion/clients/cmdline/import-cmd.c	(revision 6108)
+++ subversion/clients/cmdline/import-cmd.c	(working copy)
@@ -44,36 +44,33 @@
   apr_array_header_t *targets;
   const char *path;
   const char *url;
-  const char *new_entry;
   svn_client_commit_info_t *commit_info = NULL;
 
-  /* Import takes up to three arguments, for example
+  /* Import takes two arguments, for example
    *
-   *   $ svn import  file:///home/jrandom/repos  ./myproj  myproj
-   *                 ^^^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^  ^^^^^^
-   *                        (repository)          (source)  (dest)
+   *   $ svn import projects/test file:///home/jrandom/repos/trunk
+   *                ^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   *                 (source)       (repository)
    *
    * or
    *
-   *   $ svn import  file:///home/jrandom/repos/some/subdir  .  myproj
+   *   $ svn import file:///home/jrandom/repos/some/subdir
    *
    * What is the nicest behavior for import, from the user's point of
    * view?  This is a subtle question.  Seemingly intuitive answers
    * can lead to weird situations, such never being able to create
    * non-directories in the top-level of the repository.
    *
-   * For now, let's keep things simple:
+   * If 'source' is a file then the basename of 'url' is used as the
+   * filename in the repository.  If 'source' is a directory then the
+   * import happens directly in the repository target dir, creating
+   * however many new entries are necessary.  If some part of 'url'
+   * does not exist in the repository then parent directories are created
+   * as necessary.
    *
-   * If the third arg is present, it is the name of the new entry in
-   * the repository target dir (the latter may or may not be the root
-   * dir).  If it is absent, then the import happens directly in the
-   * repository target dir, creating however many new entries are
-   * necessary.
+   * In the case where no 'source' is given '.' (the current directory)
+   * is implied.
    *
-   * If the second arg is also omitted, then "." is implied.
-   *
-   * The first arg cannot be omitted, of course.
-   *
    * ### kff todo: review above behaviors.
    */
 
@@ -83,31 +80,30 @@
                                          &(opt_state->end_revision),
                                          FALSE, pool));
 
-  /* Get a repository url. */
   if (targets->nelts < 1)
     return svn_error_create
       (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
        "repository url required when importing");
-  else
-    url = ((const char **) (targets->elts))[0];
-
-  /* Get a local path. */
-  if (targets->nelts < 2)
-    path = "";
-  else
-    path = ((const char **) (targets->elts))[1];
-
-  /* Optionally get the dest entry name. */
-  if (targets->nelts < 3)
-    new_entry = NULL;  /* tells import() to create many entries at top
-                          level. */
-  else if (targets->nelts == 3)
-    new_entry = ((const char **) (targets->elts))[2];
-  else
+  else if (targets->nelts > 2)
     return svn_error_create
       (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
        "too many arguments to import command");
-  
+  else if (targets->nelts == 1)
+    {
+      url = ((const char **) (targets->elts))[0];
+      path = "";
+    }
+  else
+    {
+      path = ((const char **) (targets->elts))[0];
+      url = ((const char **) (targets->elts))[1];
+    }
+
+  if (! svn_path_is_url (url))
+    return svn_error_createf
+      (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+       "invalid url '%s'", url);
+
   if (! opt_state->quiet)
     svn_cl__get_notifier (&ctx->notify_func, &ctx->notify_baton,
                           FALSE, FALSE, FALSE, pool);
@@ -118,7 +114,6 @@
            (ctx->log_msg_baton, svn_client_import (&commit_info,
                                                    path,
                                                    url,
-                                                   new_entry,
                                                    opt_state->nonrecursive,
                                                    ctx,
                                                    pool)));
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c	(revision 6108)
+++ subversion/clients/cmdline/main.c	(working copy)
@@ -247,11 +247,10 @@
   
   { "import", svn_cl__import, {0},
     "Commit an unversioned file or tree into the repository.\n"
-    "usage: import URL [PATH [NEW_ENTRY_IN_REPOS]]\n\n"
+    "usage: import [PATH] URL\n\n"
     "  Recursively commit a copy of PATH to URL.\n"
-    "  If no third arg, copy top-level contents of PATH into URL\n"
-    "  directly.  Otherwise, create NEW_ENTRY underneath URL and\n"
-    "  begin copy there.\n",
+    "  If PATH is omitted '.' is assumed.  Parent directories are created\n"
+    "  as necessary in the repository.\n",
     {'m', 'F', 'q', 'N', SVN_CL__AUTH_OPTIONS,
      svn_cl__editor_cmd_opt, svn_cl__encoding_opt} },
  
Index: subversion/tests/clients/cmdline/basic_tests.py
===================================================================
--- subversion/tests/clients/cmdline/basic_tests.py	(revision 6108)
+++ subversion/tests/clients/cmdline/basic_tests.py	(working copy)
@@ -1143,12 +1143,12 @@
   svntest.main.file_append(new_path, "some text")
 
   # import new files into repository
-  url = svntest.main.current_repo_url
+  url = os.path.join(svntest.main.current_repo_url, "dirA/dirB/new_file")
   output, errput =   svntest.actions.run_and_verify_svn(
     'Cannot change node kind', None, [], 'import',
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
-    '-m', 'Log message for new import', url, new_path, 'new_file')
+    '-m', 'Log message for new import', new_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
@@ -1163,19 +1163,23 @@
   # Create expected disk tree for the update (disregarding props)
   expected_disk = svntest.main.greek_state.copy()
   expected_disk.add({
-    'new_file' : Item('some text'),
+    'dirA/dirB/new_file' : Item('some text'),
     })
 
   # Create expected status tree for the update (disregarding props).
   # Newly imported file should be at revision 2.
   expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
   expected_status.add({
-    'new_file' : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA'                : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA/dirB'           : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA/dirB//new_file' : Item(status='  ', wc_rev=2, repos_rev=2),
     })
 
   # Create expected output tree for the update.
   expected_output = svntest.wc.State(wc_dir, {
-    'new_file' : Item(status='A '),
+    'dirA'               : Item(status='A '),
+    'dirA/dirB'          : Item(status='A '),
+    'dirA/dirB/new_file' : Item(status='A '),
   })
 
   # do update and check three ways
@@ -1221,7 +1225,7 @@
     None, None, [], 'import',
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
-    '-m', 'Log message for new import', url, xt_path)
+    '-m', 'Log message for new import', xt_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
@@ -1483,7 +1487,7 @@
   open(foo_o_path, 'w')
 
   # import new dir into repository
-  url = svntest.main.current_repo_url
+  url = os.path.join(svntest.main.current_repo_url, 'dir')
 
 
   output, errput = svntest.actions.run_and_verify_svn(
@@ -1491,7 +1495,7 @@
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
     '-m', 'Log message for new import',
-    url, dir_path, 'dir')
+    dir_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
Index: subversion/tests/clients/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/clients/cmdline/svntest/actions.py	(revision 6108)
+++ subversion/tests/clients/cmdline/svntest/actions.py	(working copy)
@@ -78,7 +78,7 @@
                                   '--username', main.wc_author,
                                   '--password', main.wc_passwd,
                                   '-m', 'Log message for revision 1.',
-                                  url, main.greek_dump_dir)
+                                  main.greek_dump_dir, url)
 
     # check for any errors from the import
     if len(errput):
Index: doc/book/book/ch03.xml
===================================================================
--- doc/book/book/ch03.xml	(revision 6108)
+++ doc/book/book/ch03.xml	(working copy)
@@ -1972,11 +1972,9 @@
       <para>The import command is a quick way to move an unversioned
         tree of files into a repository.</para>
 
-      <para>There are two ways to use this command:</para>
-
       <screen>
 $ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree
+$ svn import mytree file:///usr/local/svn/newrepos/fooproject
 Adding  mytree/foo.c
 Adding  mytree/bar.c
 Adding  mytree/subdir
@@ -1986,34 +1984,10 @@
       </screen>
 
       <para>The above example places the contents of directory
-        <filename>mytree</filename> directly into the root of the
-        repository:</para>
+        <filename>mytree</filename> under the directory
+        <filename>fooproject</filename> in the repository:</para>
 
       <screen>
-/foo.c
-/bar.c
-/subdir
-/subdir/quux.h
-      </screen>
-
-      <para>If you give <command>svn import</command> a third
-        argument, it will use the argument as the name of a new
-        subdirectory to create within the URL.</para>
-
-      <screen>
-$ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree fooproject
-Adding  mytree/foo.c
-Adding  mytree/bar.c
-Adding  mytree/subdir
-Adding  mytree/subdir/quux.h
-Transmitting file data....
-Committed revision 1.
-      </screen>
-
-      <para>The repository would now look like this:</para>
-
-      <screen>
 /fooproject/foo.c
 /fooproject/bar.c
 /fooproject/subdir
Index: doc/book/book/ch05.xml
===================================================================
--- doc/book/book/ch05.xml	(revision 6108)
+++ doc/book/book/ch05.xml	(working copy)
@@ -2654,7 +2654,7 @@
 $ mkdir projectB/trunk
 $ mkdir projectB/branches
 &hellip;
-$ svn import file:///path/to/repos . --message 'Initial repository layout'
+$ svn import file:///path/to/repos --message 'Initial repository layout'
 Adding         projectA
 Adding         projectA/trunk
 Adding         projectA/branches
Index: doc/book/book/ch06.xml
===================================================================
--- doc/book/book/ch06.xml	(revision 6108)
+++ doc/book/book/ch06.xml	(working copy)
@@ -1679,13 +1679,9 @@
         first vendor drop.</para>
 
       <screen>
-$ svn mkdir http://svn.example.com/repos/calc/vendor/libcomplex  \
-            -m 'creating vendor branch area'
-
 &hellip;
-$ svn import http://svn.example.com/repos/calc/vendor/libcomplex \
-             /path/to/libcomplex-1.0                             \
-             current                                             \
+$ svn import /path/to/libcomplex-1.0 \
+             http://svn.example.com/repos/calc/vendor/libcomplex/current \
              -m 'importing initial 1.0 vendor drop'
 &hellip;
 </screen>
Index: doc/book/book/ch08.xml
===================================================================
--- doc/book/book/ch08.xml	(revision 6108)
+++ doc/book/book/ch08.xml	(working copy)
@@ -1306,18 +1306,15 @@
           </refnamediv>
           <refsect1>
             <title>Synopsis</title>
-            <programlisting>svn import URL [PATH [NEW_ENTRY_IN_REPOS]]</programlisting>
+            <programlisting>svn import [PATH] URL</programlisting>
           </refsect1>
           <refsect1>
             <title>Description</title>
 
             <para>Recursively commit a copy of <literal>PATH</literal>
-              to <literal>URL</literal>.  If there is no third
-              argument, copy the top-level contents of
-              <literal>PATH</literal> into <literal>URL</literal>
-              directly.  Otherwise, create
-              <literal>NEW_ENTRY</literal> underneath
-              <literal>URL</literal> and begin copy there.</para>
+              to <literal>URL</literal>.  If <literal>PATH</literal>
+              is omitted '.' is assumed.  Parent directories are created
+              in the repository as necessary.</para>
           </refsect1>
 
           <refsect1>
@@ -1358,7 +1355,7 @@
               root of your repository:</para>
 
             <screen>
-$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj
+$ svn import -m "New import" myproj http://svn.red-bean.com/repos/test
 Adding         myproj/sample.txt
 ...
 Transmitting file data .........
@@ -1367,12 +1364,11 @@
 
             <para>This imports the local directory 'myproj' into
               'trunk/vendors' in your repository.  The directory
-              'trunk/vendors' must exist before you import into
-              it&mdash;<command>svn import</command> will not
+              'trunk/vendors' need not exist before you import into
+              it&mdash;<command>svn import</command> will
               recursively create directories for you:</para>
-
             <screen>
-$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj trunk/vendors/myproj
+$ svn import -m "New import" myproj http://svn.red-bean.com/repos/test/trunk/vendors/myproj
 Adding         myproj/sample.txt
 ...
 Transmitting file data .........

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: (resend) [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by cm...@collab.net.
David Kimdon <dw...@debian.org> writes:

> Super, here is a revised patch, testsuite run over file: and svn:
> access methods.

I made a couple of tweaks (stuff I missed in my last review), and now
I'm testing the result.  The tweaks were really simple: in the
do/while() loop for opening the RA session, we needed to call
svn_error_clear() on non-fatal errors.  And then there were some
places where you didn't follow the space-before-paren coding style
(where what should have been 'fn_call (param)' was 'fn_call(param)').
Not really big deals to fix.

Okay, testing looks pretty good.  Now, after having used the feature,
I have the following complaints:

   - I'm not really fond of the fact that there is no notification
     regarding the intermediate directories.  Sucker just sits there
     (especially long over ra_dav ... like everything else over
     ra_dav) appearing to do nothing.  I dunno the fix for this,
     though, because we don't want the notifications for intermediate
     dirs to appear as though they were on-disk.

   - The generated log message also doesn't show the creation of the
     intermediate directories.  Not fond of this, either, but again,
     the solution evades me.

   - We need to be calling the cancel_func() during the do/while()
     loop in case the user has had a change of heart.

I'll take care of the cancel_func() thing, and will go ahead and
commit this all up if it passes testing.  Thanks for the work, David.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: (resend) [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by cm...@collab.net.
David Kimdon <dw...@debian.org> writes:

> Super, here is a revised patch, testsuite run over file: and svn:
> access methods.

Thanks, David.  Just letting you know that I haven't overlooked this
patch review.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: (resend) [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by David Kimdon <dw...@debian.org>.
On Wed, Jun 11, 2003 at 01:07:39AM -0500, cmpilato@collab.net wrote:
> Ah.  Now I see why the RA->check_path() was used only once.  Man, to
> tell you the truth, I hate this inelegance of this -- but, I also feel
> like this is the only real way to solve the problem.  

That makes two of us, I poked around in the fs code for a while to try
and make a cleaner way of doing this 'mkdir -p' operation but it was
looking like a bigger patch than was reasonable.  The fs API has
enough for us to do what we want already.

> By clearing the subpool after each failure, you are
> shutting down the defunct RA session and all the sludge associated
> with it.

done.

> > +    'dirA/dirB//new_file' : Item(status='  ', wc_rev=2, repos_rev=2),
> Oops.  ----------^  Double-slash in there.

got that too.

> I haven't applied your patch for real testing.  But the above comments
> are based on a quick read-thru.  I'd be happy to test a revised patch
> once you submit one.

Super, here is a revised patch, testsuite run over file: and svn:
access methods.

-David


Fix issue #933: svn import syntax is confusing; merge 1st and 3rd arguments
Fix issue #735: import should 'mkdir -p'

Import syntax is now:

svn import [PATH] URL

Parent directories in URL are created as necessary.  Testsuite and
documentation updated as well.

* subversion/include/svn_client.h (svn_client_import) : Remove 'new_entry'
  argument.  Trailing entries in URL that do not exist in the repository
  are considered to be a new path to create.
* subversion/libsvn_client/commit.c
  (import) :  Interpret 'new_entry' as a path to be created in the repository
  within which the actual import will take place.
  (get_ra_editor) : Return an error if 'base_url' does not correspond to a 
  repository path.
  (svn_client_import) : If the first attempt to open a repository session
  fails due to a non-existent URL, back up a directory and try again.  This
  allows us to create 'new_entry' dynamically rather than requiring an 
  argument.
* subversion/clients/cmdline/import-cmd.c : 'import' now takes at most
  two rather than three arguments.  PATH now comes before URL.
* subversion/tests/clients/cmdline/basic_tests.py : Update single file
  import test so parent directories are created to hold the actual
  import.


Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h	(revision 6226)
+++ subversion/include/svn_client.h	(working copy)
@@ -573,24 +573,18 @@
  * head, authenticating with the authentication baton cached in @a ctx, 
  * and using @a ctx->log_msg_func/@a ctx->log_msg_baton to get a log message 
  * for the (implied) commit.  Set @a *commit_info to the results of the 
- * commit, allocated in @a pool.
+ * commit, allocated in @a pool.  If some components of @a url do not exist
+ * then create parent directories as necessary.
  *
- * @a new_entry is the new entry created in the repository directory
- * identified by @a url.  @a new_entry may be null (see below), but may 
- * not be the empty string.
- *
  * If @a path is a directory, the contents of that directory are
- * imported, under a new directory named @a new_entry under @a url; or 
- * if @a new_entry is null, then the contents of @a path are imported 
- * directly into the directory identified by @a url.  Note that the 
+ * imported directly into the directory identified by @a url.  Note that the
  * directory @a path itself is not imported -- that is, the basename of 
  * @a path is not part of the import.
  *
- * If @a path is a file, that file is imported as @a new_entry (which may
- * not be @c NULL).
+ * If @a path is a file, then the dirname of @a url is the directory
+ * receiving the import.  The basename of @a url is the filename in the
+ * repository.  In this case if @a url already exists, return error.
  *
- * In all cases, if @a new_entry already exists in @a url, return error.
- * 
  * If @a ctx->notify_func is non-null, then call @a ctx->notify_func with 
  * @a ctx->notify_baton as the import progresses, with any of the following 
  * actions: @c svn_wc_notify_commit_added,
@@ -614,7 +608,6 @@
 svn_error_t *svn_client_import (svn_client_commit_info_t **commit_info,
                                 const char *path,
                                 const char *url,
-                                const char *new_entry,
                                 svn_boolean_t nonrecursive,
                                 svn_client_ctx_t *ctx,
                                 apr_pool_t *pool);
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c	(revision 6226)
+++ subversion/libsvn_client/commit.c	(working copy)
@@ -311,7 +311,9 @@
  * in the top repository target directory as there are entries in the
  * top of PATH; but if NEW_ENTRY is non-null, it is the name of a new
  * subdirectory in the repository to hold the import.  If PATH is a
- * file, NEW_ENTRY may not be null.
+ * file, NEW_ENTRY may not be null.  NEW_ENTRY can have multiple
+ * components in which case parent directories are created as
+ * needed. 
  * 
  * NEW_ENTRY can never be the empty string.
  * 
@@ -341,6 +343,7 @@
   void *root_baton;
   svn_node_kind_t kind;
   apr_array_header_t *ignores;
+  apr_array_header_t *batons = NULL;
 
   /* Get a root dir baton.  We pass an invalid revnum to open_root
      to mean "base this on the youngest revision".  Should we have an
@@ -351,6 +354,40 @@
   /* Import a file or a directory tree. */
   SVN_ERR (svn_io_check_path (path, &kind, pool));
 
+  if (new_entry)
+    {
+      apr_array_header_t *dirs;
+      const char *new_path = "";
+      int i;
+
+      dirs = svn_path_decompose (new_entry, pool);
+
+      /* If we are importing a file then NEW_ENTRY's basename is
+       * the desired filename in the repository.  We don't create
+       * a directory with that name. */
+      if (kind == svn_node_file)
+        apr_array_pop (dirs);
+
+      for (i = 0; i < dirs->nelts; i++)
+        {
+          void *temp;
+
+          if (!batons)
+            batons = apr_array_make (pool, 1, sizeof (void *));
+
+          *((void **) apr_array_push (batons)) = root_baton;
+
+          new_path = svn_path_join (new_path, ((char **)dirs->elts)[i],
+                                    pool);
+
+          SVN_ERR (editor->add_directory (new_path,
+                                          root_baton,
+                                          NULL, SVN_INVALID_REVNUM,
+                                          pool, &temp));
+          root_baton = temp;
+        }
+    }
+
   /* Note that there is no need to check whether PATH's basename is
      the same name that we reserve for our admistritave
      subdirectories.  It would be strange, but not illegal to import
@@ -374,14 +411,6 @@
     }
   else if (kind == svn_node_dir)
     {
-      void *new_dir_baton = NULL;
-
-      /* Grab a new baton, making two we'll have to close. */
-      if (new_entry)
-        SVN_ERR (editor->add_directory (new_entry, root_baton,
-                                        NULL, SVN_INVALID_REVNUM,
-                                        pool, &new_dir_baton));
-
 #if 0 /* Temporarily blocked out for consideration, see below. */
       /* If we activate this notification, then
        *
@@ -415,13 +444,10 @@
 #endif /* 0 */
 
       SVN_ERR (import_dir (editor, 
-                           new_dir_baton ? new_dir_baton : root_baton, 
+                           root_baton,
                            path, new_entry ? new_entry : "",
                            nonrecursive, excludes, ctx, pool));
 
-      /* Close one baton or two. */
-      if (new_dir_baton)
-        SVN_ERR (editor->close_directory (new_dir_baton, pool));
     }
   else if (kind == svn_node_none)
     {
@@ -432,6 +458,15 @@
 
   /* Close up the show; it's time to go home. */
   SVN_ERR (editor->close_directory (root_baton, pool));
+
+  if (batons)
+    {
+      void **baton;
+
+      while ((baton = (void **) apr_array_pop (batons)))
+        SVN_ERR (editor->close_directory (*baton, pool));
+    }
+
   SVN_ERR (editor->close_edit (edit_baton, pool));
 
   return SVN_NO_ERROR;
@@ -469,6 +504,19 @@
                                         is_commit, !is_commit,
                                         ctx, pool));
 
+  if (! is_commit)
+    {
+      svn_node_kind_t kind;
+ 
+      SVN_ERR ((*ra_lib)->check_path (&kind, *session, "", SVN_INVALID_REVNUM, 
+                                      pool));
+
+      if (kind == svn_node_none)
+        return svn_error_createf (SVN_ERR_FS_NO_SUCH_ENTRY, NULL,
+                                  "the path '%s' does not exist",
+                                  base_url);
+    }
+
   /* Fetch the latest revision if requested. */
   if (latest_rev)
     SVN_ERR ((*ra_lib)->get_latest_revnum (*session, latest_rev, pool));
@@ -486,7 +534,6 @@
 svn_client_import (svn_client_commit_info_t **commit_info,
                    const char *path,
                    const char *url,
-                   const char *new_entry,
                    svn_boolean_t nonrecursive,
                    svn_client_ctx_t *ctx,
                    apr_pool_t *pool)
@@ -501,20 +548,9 @@
   const char *committed_date = NULL;
   const char *committed_author = NULL;
   apr_hash_t *excludes = apr_hash_make (pool);
+  const char *new_entry = NULL;
 
-  /* Sanity check: NEW_ENTRY can be null or non-empty, but it can't be
-     empty. */
-  if (new_entry && (strcmp (new_entry, "") == 0))
-    return svn_error_create (SVN_ERR_FS_PATH_SYNTAX, NULL,
-                             "empty string is an invalid entry name");
 
-  /* The repository doesn't know about the reserved. */
-  if (new_entry && strcmp (new_entry, SVN_WC_ADM_DIR_NAME) == 0)
-    return svn_error_createf
-      (SVN_ERR_CL_ADM_DIR_RESERVED, NULL,
-       "the name \"%s\" is reserved and cannot be imported",
-       SVN_WC_ADM_DIR_NAME);
-
   /* Create a new commit item and add it to the array. */
   if (ctx->log_msg_func)
     {
@@ -550,18 +586,75 @@
     {
       svn_node_kind_t kind;
       const char *base_dir = path;
+      apr_array_header_t *dirs = NULL;
+      const char *temp;
+      const char *dir;
+      apr_pool_t *subpool;
 
       SVN_ERR (svn_io_check_path (path, &kind, pool));
       if (kind == svn_node_file)
         svn_path_split (path, &base_dir, NULL, pool);
 
-      SVN_ERR (get_ra_editor (&ra_baton, &session, &ra_lib, NULL,
-                              &editor, &edit_baton, ctx, url, base_dir,
-                              NULL, log_msg, NULL, &committed_rev,
-                              &committed_date, &committed_author, 
-                              FALSE, pool));
+      err = NULL;
+      subpool = svn_pool_create (pool);
+
+      do
+        {
+          svn_pool_clear (subpool);
+
+          if (err)
+            {
+              /* If get_ra_editor below failed we either tried to open
+               * an invalid url, or else some other kind of error.  In case
+               * the url was bad we back up a directory and try again. */
+
+              if (err->apr_err != SVN_ERR_FS_NO_SUCH_ENTRY)
+                return err;
+
+              if (!dirs)
+                dirs = apr_array_make (pool, 1, sizeof(const char *));
+
+              svn_path_split (url, &temp, &dir, pool);
+              *((const char **)apr_array_push (dirs)) = dir;
+              url = temp;
+            }
+        }
+      while ((err = get_ra_editor (&ra_baton, &session, &ra_lib, NULL,
+                                   &editor, &edit_baton, ctx, url, base_dir,
+                                   NULL, log_msg, NULL, &committed_rev,
+                                   &committed_date, &committed_author,
+                                   FALSE, subpool)));
+
+      if (dirs)
+        {
+          const char **child;
+
+          if ((child = (const char **) apr_array_pop(dirs)))
+            {
+              new_entry = *child;
+
+              while ((child = (const char **) apr_array_pop(dirs)))
+                new_entry = svn_path_join (new_entry, *child, pool);
+            }
+        }
+
+      /* new_entry == NULL means the first call to get_ra_editor
+       * above succeeded.  That means that 'url' corresponds to an
+       * already existing filesystem entity. */
+      if (kind == svn_node_file && ! new_entry)
+        return svn_error_createf
+          (SVN_ERR_ENTRY_EXISTS, NULL,
+           "the path \"%s\" already exists", url);
     }
 
+  /* The repository doesn't know about the reserved. */
+  if (new_entry && strcmp (new_entry, SVN_WC_ADM_DIR_NAME) == 0)
+    return svn_error_createf
+      (SVN_ERR_CL_ADM_DIR_RESERVED, NULL,
+       "the name \"%s\" is reserved and cannot be imported",
+       SVN_WC_ADM_DIR_NAME);
+
+
   /* If an error occured during the commit, abort the edit and return
      the error.  We don't even care if the abort itself fails.  */
   if ((err = import (path, new_entry,
Index: subversion/clients/cmdline/man/svn.1
===================================================================
--- subversion/clients/cmdline/man/svn.1	(revision 6226)
+++ subversion/clients/cmdline/man/svn.1	(working copy)
@@ -220,7 +220,7 @@
 command is entered, a short description on how to use that command is
 presented.
 .TP
-\fBimport\fP \fIRepository-URL\fP [\fIPath\fP] [\fINew-Repository-Entry\fP] [\fI--username name\fP] [\fI--password pass\fP] [\fI--no-auth-cache\fP] [\fI--encoding\fP] [\fI-FmqN\fP]
+\fBimport\fP [\fIPath\fP] \fIRepository-URL\fP [\fI--username name\fP] [\fI--password pass\fP] [\fI--no-auth-cache\fP] [\fI--encoding\fP] [\fI-FmqN\fP]
 Import a file or tree into the repository.
 .TP
 \fBinfo\fP \fItarget1\fP [\fItarget2\fP ...] [\fI--targets file\fP] [\fI-R\fP]
Index: subversion/clients/cmdline/import-cmd.c
===================================================================
--- subversion/clients/cmdline/import-cmd.c	(revision 6226)
+++ subversion/clients/cmdline/import-cmd.c	(working copy)
@@ -44,36 +44,33 @@
   apr_array_header_t *targets;
   const char *path;
   const char *url;
-  const char *new_entry;
   svn_client_commit_info_t *commit_info = NULL;
 
-  /* Import takes up to three arguments, for example
+  /* Import takes two arguments, for example
    *
-   *   $ svn import  file:///home/jrandom/repos  ./myproj  myproj
-   *                 ^^^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^  ^^^^^^
-   *                        (repository)          (source)  (dest)
+   *   $ svn import projects/test file:///home/jrandom/repos/trunk
+   *                ^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   *                 (source)       (repository)
    *
    * or
    *
-   *   $ svn import  file:///home/jrandom/repos/some/subdir  .  myproj
+   *   $ svn import file:///home/jrandom/repos/some/subdir
    *
    * What is the nicest behavior for import, from the user's point of
    * view?  This is a subtle question.  Seemingly intuitive answers
    * can lead to weird situations, such never being able to create
    * non-directories in the top-level of the repository.
    *
-   * For now, let's keep things simple:
+   * If 'source' is a file then the basename of 'url' is used as the
+   * filename in the repository.  If 'source' is a directory then the
+   * import happens directly in the repository target dir, creating
+   * however many new entries are necessary.  If some part of 'url'
+   * does not exist in the repository then parent directories are created
+   * as necessary.
    *
-   * If the third arg is present, it is the name of the new entry in
-   * the repository target dir (the latter may or may not be the root
-   * dir).  If it is absent, then the import happens directly in the
-   * repository target dir, creating however many new entries are
-   * necessary.
+   * In the case where no 'source' is given '.' (the current directory)
+   * is implied.
    *
-   * If the second arg is also omitted, then "." is implied.
-   *
-   * The first arg cannot be omitted, of course.
-   *
    * ### kff todo: review above behaviors.
    */
 
@@ -83,31 +80,30 @@
                                          &(opt_state->end_revision),
                                          FALSE, pool));
 
-  /* Get a repository url. */
   if (targets->nelts < 1)
     return svn_error_create
       (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
        "repository url required when importing");
-  else
-    url = ((const char **) (targets->elts))[0];
-
-  /* Get a local path. */
-  if (targets->nelts < 2)
-    path = "";
-  else
-    path = ((const char **) (targets->elts))[1];
-
-  /* Optionally get the dest entry name. */
-  if (targets->nelts < 3)
-    new_entry = NULL;  /* tells import() to create many entries at top
-                          level. */
-  else if (targets->nelts == 3)
-    new_entry = ((const char **) (targets->elts))[2];
-  else
+  else if (targets->nelts > 2)
     return svn_error_create
       (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
        "too many arguments to import command");
-  
+  else if (targets->nelts == 1)
+    {
+      url = ((const char **) (targets->elts))[0];
+      path = "";
+    }
+  else
+    {
+      path = ((const char **) (targets->elts))[0];
+      url = ((const char **) (targets->elts))[1];
+    }
+
+  if (! svn_path_is_url (url))
+    return svn_error_createf
+      (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+       "invalid url '%s'", url);
+
   if (! opt_state->quiet)
     svn_cl__get_notifier (&ctx->notify_func, &ctx->notify_baton,
                           FALSE, FALSE, FALSE, pool);
@@ -118,7 +114,6 @@
            (ctx->log_msg_baton, svn_client_import (&commit_info,
                                                    path,
                                                    url,
-                                                   new_entry,
                                                    opt_state->nonrecursive,
                                                    ctx,
                                                    pool)));
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c	(revision 6226)
+++ subversion/clients/cmdline/main.c	(working copy)
@@ -248,11 +248,10 @@
   
   { "import", svn_cl__import, {0},
     "Commit an unversioned file or tree into the repository.\n"
-    "usage: import URL [PATH [NEW_ENTRY_IN_REPOS]]\n\n"
+    "usage: import [PATH] URL\n\n"
     "  Recursively commit a copy of PATH to URL.\n"
-    "  If no third arg, copy top-level contents of PATH into URL\n"
-    "  directly.  Otherwise, create NEW_ENTRY underneath URL and\n"
-    "  begin copy there.\n",
+    "  If PATH is omitted '.' is assumed.  Parent directories are created\n"
+    "  as necessary in the repository.\n",
     {'m', 'F', 'q', 'N', SVN_CL__AUTH_OPTIONS,
      svn_cl__editor_cmd_opt, svn_cl__encoding_opt} },
  
Index: subversion/tests/clients/cmdline/basic_tests.py
===================================================================
--- subversion/tests/clients/cmdline/basic_tests.py	(revision 6226)
+++ subversion/tests/clients/cmdline/basic_tests.py	(working copy)
@@ -1143,12 +1143,12 @@
   svntest.main.file_append(new_path, "some text")
 
   # import new files into repository
-  url = svntest.main.current_repo_url
+  url = os.path.join(svntest.main.current_repo_url, "dirA/dirB/new_file")
   output, errput =   svntest.actions.run_and_verify_svn(
     'Cannot change node kind', None, [], 'import',
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
-    '-m', 'Log message for new import', url, new_path, 'new_file')
+    '-m', 'Log message for new import', new_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
@@ -1163,19 +1163,23 @@
   # Create expected disk tree for the update (disregarding props)
   expected_disk = svntest.main.greek_state.copy()
   expected_disk.add({
-    'new_file' : Item('some text'),
+    'dirA/dirB/new_file' : Item('some text'),
     })
 
   # Create expected status tree for the update (disregarding props).
   # Newly imported file should be at revision 2.
   expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
   expected_status.add({
-    'new_file' : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA'                : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA/dirB'           : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA/dirB/new_file' : Item(status='  ', wc_rev=2, repos_rev=2),
     })
 
   # Create expected output tree for the update.
   expected_output = svntest.wc.State(wc_dir, {
-    'new_file' : Item(status='A '),
+    'dirA'               : Item(status='A '),
+    'dirA/dirB'          : Item(status='A '),
+    'dirA/dirB/new_file' : Item(status='A '),
   })
 
   # do update and check three ways
@@ -1221,7 +1225,7 @@
     None, None, [], 'import',
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
-    '-m', 'Log message for new import', url, xt_path)
+    '-m', 'Log message for new import', xt_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
@@ -1483,7 +1487,7 @@
   open(foo_o_path, 'w')
 
   # import new dir into repository
-  url = svntest.main.current_repo_url
+  url = os.path.join(svntest.main.current_repo_url, 'dir')
 
 
   output, errput = svntest.actions.run_and_verify_svn(
@@ -1491,7 +1495,7 @@
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
     '-m', 'Log message for new import',
-    url, dir_path, 'dir')
+    dir_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
Index: subversion/tests/clients/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/clients/cmdline/svntest/actions.py	(revision 6226)
+++ subversion/tests/clients/cmdline/svntest/actions.py	(working copy)
@@ -78,7 +78,7 @@
                                   '--username', main.wc_author,
                                   '--password', main.wc_passwd,
                                   '-m', 'Log message for revision 1.',
-                                  url, main.greek_dump_dir)
+                                  main.greek_dump_dir, url)
 
     # check for any errors from the import
     if len(errput):
Index: doc/book/book/ch03.xml
===================================================================
--- doc/book/book/ch03.xml	(revision 6226)
+++ doc/book/book/ch03.xml	(working copy)
@@ -1972,11 +1972,9 @@
       <para>The import command is a quick way to move an unversioned
         tree of files into a repository.</para>
 
-      <para>There are two ways to use this command:</para>
-
       <screen>
 $ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree
+$ svn import mytree file:///usr/local/svn/newrepos/fooproject
 Adding  mytree/foo.c
 Adding  mytree/bar.c
 Adding  mytree/subdir
@@ -1986,34 +1984,10 @@
       </screen>
 
       <para>The above example places the contents of directory
-        <filename>mytree</filename> directly into the root of the
-        repository:</para>
+        <filename>mytree</filename> under the directory
+        <filename>fooproject</filename> in the repository:</para>
 
       <screen>
-/foo.c
-/bar.c
-/subdir
-/subdir/quux.h
-      </screen>
-
-      <para>If you give <command>svn import</command> a third
-        argument, it will use the argument as the name of a new
-        subdirectory to create within the URL.</para>
-
-      <screen>
-$ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree fooproject
-Adding  mytree/foo.c
-Adding  mytree/bar.c
-Adding  mytree/subdir
-Adding  mytree/subdir/quux.h
-Transmitting file data....
-Committed revision 1.
-      </screen>
-
-      <para>The repository would now look like this:</para>
-
-      <screen>
 /fooproject/foo.c
 /fooproject/bar.c
 /fooproject/subdir
Index: doc/book/book/ch05.xml
===================================================================
--- doc/book/book/ch05.xml	(revision 6226)
+++ doc/book/book/ch05.xml	(working copy)
@@ -2735,7 +2735,7 @@
 $ mkdir projectB/trunk
 $ mkdir projectB/branches
 &hellip;
-$ svn import file:///path/to/repos . --message 'Initial repository layout'
+$ svn import file:///path/to/repos --message 'Initial repository layout'
 Adding         projectA
 Adding         projectA/trunk
 Adding         projectA/branches
Index: doc/book/book/ch06.xml
===================================================================
--- doc/book/book/ch06.xml	(revision 6226)
+++ doc/book/book/ch06.xml	(working copy)
@@ -1675,13 +1675,9 @@
         first vendor drop.</para>
 
       <screen>
-$ svn mkdir http://svn.example.com/repos/calc/vendor/libcomplex  \
-            -m 'creating vendor branch area'
-
 &hellip;
-$ svn import http://svn.example.com/repos/calc/vendor/libcomplex \
-             /path/to/libcomplex-1.0                             \
-             current                                             \
+$ svn import /path/to/libcomplex-1.0 \
+             http://svn.example.com/repos/calc/vendor/libcomplex/current \
              -m 'importing initial 1.0 vendor drop'
 &hellip;
 </screen>
Index: doc/book/book/ch08.xml
===================================================================
--- doc/book/book/ch08.xml	(revision 6226)
+++ doc/book/book/ch08.xml	(working copy)
@@ -1306,18 +1306,15 @@
           </refnamediv>
           <refsect1>
             <title>Synopsis</title>
-            <programlisting>svn import URL [PATH [NEW_ENTRY_IN_REPOS]]</programlisting>
+            <programlisting>svn import [PATH] URL</programlisting>
           </refsect1>
           <refsect1>
             <title>Description</title>
 
             <para>Recursively commit a copy of <literal>PATH</literal>
-              to <literal>URL</literal>.  If there is no third
-              argument, copy the top-level contents of
-              <literal>PATH</literal> into <literal>URL</literal>
-              directly.  Otherwise, create
-              <literal>NEW_ENTRY</literal> underneath
-              <literal>URL</literal> and begin copy there.</para>
+              to <literal>URL</literal>.  If <literal>PATH</literal>
+              is omitted '.' is assumed.  Parent directories are created
+              in the repository as necessary.</para>
           </refsect1>
 
           <refsect1>
@@ -1358,7 +1355,7 @@
               root of your repository:</para>
 
             <screen>
-$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj
+$ svn import -m "New import" myproj http://svn.red-bean.com/repos/test
 Adding         myproj/sample.txt
 ...
 Transmitting file data .........
@@ -1367,12 +1364,11 @@
 
             <para>This imports the local directory 'myproj' into
               'trunk/vendors' in your repository.  The directory
-              'trunk/vendors' must exist before you import into
-              it&mdash;<command>svn import</command> will not
+              'trunk/vendors' need not exist before you import into
+              it&mdash;<command>svn import</command> will
               recursively create directories for you:</para>
-
             <screen>
-$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj trunk/vendors/myproj
+$ svn import -m "New import" myproj http://svn.red-bean.com/repos/test/trunk/vendors/myproj
 Adding         myproj/sample.txt
 ...
 Transmitting file data .........
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: (resend) [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by cm...@collab.net.
David Kimdon <dw...@debian.org> writes:

> Index: subversion/libsvn_client/commit.c
> ===================================================================
> --- subversion/libsvn_client/commit.c	(revision 6178)
> +++ subversion/libsvn_client/commit.c	(working copy)

[...]

> @@ -550,18 +583,71 @@
>      {
>        svn_node_kind_t kind;
>        const char *base_dir = path;
> +      apr_array_header_t *dirs = NULL;
> +      const char *temp;
> +      const char *dir;
>  
>        SVN_ERR (svn_io_check_path (path, &kind, pool));
>        if (kind == svn_node_file)
>          svn_path_split (path, &base_dir, NULL, pool);
>  
> -      SVN_ERR (get_ra_editor (&ra_baton, &session, &ra_lib, NULL,
> -                              &editor, &edit_baton, ctx, url, base_dir,
> -                              NULL, log_msg, NULL, &committed_rev,
> -                              &committed_date, &committed_author, 
> -                              FALSE, pool));
> +      err = NULL;
> +
> +      do
> +        {
> +          if (err)
> +            {
> +              /* If get_ra_editor below failed we either tried to open
> +               * an invalid url, or else some other kind of error.  In case
> +               * the url was bad we back up a directory and try again. */
> +
> +              if (err->apr_err != SVN_ERR_FS_NO_SUCH_ENTRY)
> +                return err;
> +
> +              if (!dirs)
> +                dirs = apr_array_make (pool, 1, sizeof(const char *));
> +
> +              svn_path_split (url, &temp, &dir, pool);
> +              *((const char **)apr_array_push (dirs)) = dir;
> +              url = temp;
> +            }
> +        }
> +      while ((err = get_ra_editor (&ra_baton, &session, &ra_lib, NULL,
> +                                   &editor, &edit_baton, ctx, url, base_dir,
> +                                   NULL, log_msg, NULL, &committed_rev,
> +                                   &committed_date, &committed_author,
> +                                   FALSE, pool)));

Ah.  Now I see why the RA->check_path() was used only once.  Man, to
tell you the truth, I hate this inelegance of this -- but, I also feel
like this is the only real way to solve the problem.  One thing I'd
like to see is the use of a subpool for trying out all these
connections.  By clearing the subpool after each failure, you are
shutting down the defunct RA session and all the sludge associated
with it.

> Index: subversion/tests/clients/cmdline/basic_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/basic_tests.py	(revision 6178)
> +++ subversion/tests/clients/cmdline/basic_tests.py	(working copy)

>    # Create expected status tree for the update (disregarding props).
>    # Newly imported file should be at revision 2.
>    expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
>    expected_status.add({
> -    'new_file' : Item(status='  ', wc_rev=2, repos_rev=2),
> +    'dirA'                : Item(status='  ', wc_rev=2, repos_rev=2),
> +    'dirA/dirB'           : Item(status='  ', wc_rev=2, repos_rev=2),
> +    'dirA/dirB//new_file' : Item(status='  ', wc_rev=2, repos_rev=2),

Oops.  ----------^  Double-slash in there.

I haven't applied your patch for real testing.  But the above comments
are based on a quick read-thru.  I'd be happy to test a revised patch
once you submit one.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: (resend) [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by cm...@collab.net.
David Kimdon <dw...@debian.org> writes:

> yup, and that is what I am using.  My question is mostly, "Does this
> look safe?"  I think it looks good, but another set of eyes would be
> appreciated.  I have included the chunk in question below.
> 
> Index: subversion/libsvn_client/commit.c
> ===================================================================
> --- subversion/libsvn_client/commit.c	(revision 6178)
> +++ subversion/libsvn_client/commit.c	(working copy)
> @@ -457,6 +492,8 @@
>                 svn_boolean_t is_commit,
>                 apr_pool_t *pool)
>  {
> +  svn_node_kind_t kind;
> +
>    /* Get the RA vtable that matches URL. */
>    SVN_ERR (svn_ra_init_ra_libs (ra_baton, pool));
>    SVN_ERR (svn_ra_get_ra_library (ra_lib, *ra_baton, 
> @@ -469,6 +506,14 @@
>                                          is_commit, !is_commit,
>                                          ctx, pool));
>  
> +  SVN_ERR ((*ra_lib)->check_path (&kind, *session, "", SVN_INVALID_REVNUM, 
> +                                  pool));
> +
> +  if (kind == svn_node_none)
> +    return svn_error_createf (SVN_ERR_FS_NO_SUCH_ENTRY, NULL,
> +                              "the path '%s' does not exist",
> +                              base_url);
> +
>    /* Fetch the latest revision if requested. */
>    if (latest_rev)
>      SVN_ERR ((*ra_lib)->get_latest_revnum (*session, latest_rev, pool));

This is only for import, right?  So at the very least, you'll want to
make the RA->check_path() call conditional on (! is_commit).  We don't
want to make *any* unnecessary roundtrips to the server.  Also, I
confess to not remembering why you need the check_path() in the first
place -- but then, I haven't reviewed your entire patch.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: (resend) [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by David Kimdon <dw...@debian.org>.
On Tue, Jun 10, 2003 at 09:57:08AM -0500, cmpilato@collab.net wrote:
> David Kimdon <dw...@debian.org> writes:
> 
> > I'm resending this patch ....  I need something like 'stat()' for
> > the repository, and I believe that this extra error checking in
> > get_ra_editor() does no harm, but maybe I missed something.
> 
> Do you mean, you need an RA function that can tell you if something
> exists, and whether that something is a dir or a file?

exactly.

> Have you seen
> RA->check_path()?

yup, and that is what I am using.  My question is mostly, "Does this
look safe?"  I think it looks good, but another set of eyes would be
appreciated.  I have included the chunk in question below.

Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c	(revision 6178)
+++ subversion/libsvn_client/commit.c	(working copy)
@@ -457,6 +492,8 @@
                svn_boolean_t is_commit,
                apr_pool_t *pool)
 {
+  svn_node_kind_t kind;
+
   /* Get the RA vtable that matches URL. */
   SVN_ERR (svn_ra_init_ra_libs (ra_baton, pool));
   SVN_ERR (svn_ra_get_ra_library (ra_lib, *ra_baton, 
@@ -469,6 +506,14 @@
                                         is_commit, !is_commit,
                                         ctx, pool));
 
+  SVN_ERR ((*ra_lib)->check_path (&kind, *session, "", SVN_INVALID_REVNUM, 
+                                  pool));
+
+  if (kind == svn_node_none)
+    return svn_error_createf (SVN_ERR_FS_NO_SUCH_ENTRY, NULL,
+                              "the path '%s' does not exist",
+                              base_url);
+
   /* Fetch the latest revision if requested. */
   if (latest_rev)
     SVN_ERR ((*ra_lib)->get_latest_revnum (*session, latest_rev, pool));

> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: (resend) [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by cm...@collab.net.
David Kimdon <dw...@debian.org> writes:

> I'm resending this patch ....  I need something like 'stat()' for
> the repository, and I believe that this extra error checking in
> get_ra_editor() does no harm, but maybe I missed something.

Do you mean, you need an RA function that can tell you if something
exists, and whether that something is a dir or a file?  Have you seen
RA->check_path()?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

(resend) [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by David Kimdon <dw...@debian.org>.
Hi,

I'm resending this patch since I haven't gotten feedback on the
code in get_ra_editor().  This is basically the same patch I sent on
June 1, but I have updated a few comments and recreated the diff
against current HEAD.  I'm particularly interested in comments on the
change I made to get_ra_editor().  I need something like 'stat()' for
the repository, and I believe that this extra error checking in
get_ra_editor() does no harm, but maybe I missed something.  The
change to subversion/libsvn_client/commit.c(import) is similar to that
which has been reviewed by cmpilato previously.

-David

Fix issue #933: svn import syntax is confusing; merge 1st and 3rd arguments
Fix issue #735: import should 'mkdir -p'

Import syntax is now:

svn import [PATH] URL

Parent directories in URL are created as necessary.  Testsuite and
documentation updated as well.

* subversion/include/svn_client.h (svn_client_import) : Remove 'new_entry'
  argument.  Trailing entries in URL that do not exist in the repository
  are considered to be a new path to create.
* subversion/libsvn_client/commit.c
  (import) :  Interpret 'new_entry' as a path to be created in the repository
  within which the actual import will take place.
  (get_ra_editor) : Return an error if 'base_url' does not correspond to a 
  repository path.
  (svn_client_import) : If the first attempt to open a repository session
  fails due to a non-existent URL, back up a directory and try again.  This
  allows us to create 'new_entry' dynamically rather than requiring an 
  argument.
* subversion/clients/cmdline/import-cmd.c : 'import' now takes at most
  two rather than three arguments.  PATH now comes before URL.
* subversion/tests/clients/cmdline/basic_tests.py : Update single file
  import test so parent directories are created to hold the actual
  import.


Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h	(revision 6178)
+++ subversion/include/svn_client.h	(working copy)
@@ -573,24 +573,18 @@
  * head, authenticating with the authentication baton cached in @a ctx, 
  * and using @a ctx->log_msg_func/@a ctx->log_msg_baton to get a log message 
  * for the (implied) commit.  Set @a *commit_info to the results of the 
- * commit, allocated in @a pool.
+ * commit, allocated in @a pool.  If some components of @a url do not exist
+ * then create parent directories as necessary.
  *
- * @a new_entry is the new entry created in the repository directory
- * identified by @a url.  @a new_entry may be null (see below), but may 
- * not be the empty string.
- *
  * If @a path is a directory, the contents of that directory are
- * imported, under a new directory named @a new_entry under @a url; or 
- * if @a new_entry is null, then the contents of @a path are imported 
- * directly into the directory identified by @a url.  Note that the 
+ * imported directly into the directory identified by @a url.  Note that the
  * directory @a path itself is not imported -- that is, the basename of 
  * @a path is not part of the import.
  *
- * If @a path is a file, that file is imported as @a new_entry (which may
- * not be @c NULL).
+ * If @a path is a file, then the dirname of @a url is the directory
+ * receiving the import.  The basename of @a url is the filename in the
+ * repository.  In this case if @a url already exists, return error.
  *
- * In all cases, if @a new_entry already exists in @a url, return error.
- * 
  * If @a ctx->notify_func is non-null, then call @a ctx->notify_func with 
  * @a ctx->notify_baton as the import progresses, with any of the following 
  * actions: @c svn_wc_notify_commit_added,
@@ -614,7 +608,6 @@
 svn_error_t *svn_client_import (svn_client_commit_info_t **commit_info,
                                 const char *path,
                                 const char *url,
-                                const char *new_entry,
                                 svn_boolean_t nonrecursive,
                                 svn_client_ctx_t *ctx,
                                 apr_pool_t *pool);
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c	(revision 6178)
+++ subversion/libsvn_client/commit.c	(working copy)
@@ -311,7 +311,9 @@
  * in the top repository target directory as there are entries in the
  * top of PATH; but if NEW_ENTRY is non-null, it is the name of a new
  * subdirectory in the repository to hold the import.  If PATH is a
- * file, NEW_ENTRY may not be null.
+ * file, NEW_ENTRY may not be null.  NEW_ENTRY can have multiple
+ * components in which case parent directories are created as
+ * needed. 
  * 
  * NEW_ENTRY can never be the empty string.
  * 
@@ -341,6 +343,7 @@
   void *root_baton;
   svn_node_kind_t kind;
   apr_array_header_t *ignores;
+  apr_array_header_t *batons = NULL;
 
   /* Get a root dir baton.  We pass an invalid revnum to open_root
      to mean "base this on the youngest revision".  Should we have an
@@ -351,6 +354,40 @@
   /* Import a file or a directory tree. */
   SVN_ERR (svn_io_check_path (path, &kind, pool));
 
+  if (new_entry)
+    {
+      apr_array_header_t *dirs;
+      const char *new_path = "";
+      int i;
+
+      dirs = svn_path_decompose (new_entry, pool);
+
+      /* If we are importing a file then NEW_ENTRY's basename is
+       * the desired filename in the repository.  We don't create
+       * a directory with that name. */
+      if (kind == svn_node_file)
+        apr_array_pop (dirs);
+
+      for (i = 0; i < dirs->nelts; i++)
+        {
+          void *temp;
+
+          if (!batons)
+            batons = apr_array_make (pool, 1, sizeof (void *));
+
+          *((void **) apr_array_push (batons)) = root_baton;
+
+          new_path = svn_path_join (new_path, ((char **)dirs->elts)[i],
+                                    pool);
+
+          SVN_ERR (editor->add_directory (new_path,
+                                          root_baton,
+                                          NULL, SVN_INVALID_REVNUM,
+                                          pool, &temp));
+          root_baton = temp;
+        }
+    }
+
   /* Note that there is no need to check whether PATH's basename is
      the same name that we reserve for our admistritave
      subdirectories.  It would be strange, but not illegal to import
@@ -374,14 +411,6 @@
     }
   else if (kind == svn_node_dir)
     {
-      void *new_dir_baton = NULL;
-
-      /* Grab a new baton, making two we'll have to close. */
-      if (new_entry)
-        SVN_ERR (editor->add_directory (new_entry, root_baton,
-                                        NULL, SVN_INVALID_REVNUM,
-                                        pool, &new_dir_baton));
-
 #if 0 /* Temporarily blocked out for consideration, see below. */
       /* If we activate this notification, then
        *
@@ -415,13 +444,10 @@
 #endif /* 0 */
 
       SVN_ERR (import_dir (editor, 
-                           new_dir_baton ? new_dir_baton : root_baton, 
+                           root_baton,
                            path, new_entry ? new_entry : "",
                            nonrecursive, excludes, ctx, pool));
 
-      /* Close one baton or two. */
-      if (new_dir_baton)
-        SVN_ERR (editor->close_directory (new_dir_baton, pool));
     }
   else if (kind == svn_node_none)
     {
@@ -432,6 +458,15 @@
 
   /* Close up the show; it's time to go home. */
   SVN_ERR (editor->close_directory (root_baton, pool));
+
+  if (batons)
+    {
+      void **baton;
+
+      while ((baton = (void **) apr_array_pop (batons)))
+        SVN_ERR (editor->close_directory (*baton, pool));
+    }
+
   SVN_ERR (editor->close_edit (edit_baton, pool));
 
   return SVN_NO_ERROR;
@@ -457,6 +492,8 @@
                svn_boolean_t is_commit,
                apr_pool_t *pool)
 {
+  svn_node_kind_t kind;
+
   /* Get the RA vtable that matches URL. */
   SVN_ERR (svn_ra_init_ra_libs (ra_baton, pool));
   SVN_ERR (svn_ra_get_ra_library (ra_lib, *ra_baton, 
@@ -469,6 +506,14 @@
                                         is_commit, !is_commit,
                                         ctx, pool));
 
+  SVN_ERR ((*ra_lib)->check_path (&kind, *session, "", SVN_INVALID_REVNUM, 
+                                  pool));
+
+  if (kind == svn_node_none)
+    return svn_error_createf (SVN_ERR_FS_NO_SUCH_ENTRY, NULL,
+                              "the path '%s' does not exist",
+                              base_url);
+
   /* Fetch the latest revision if requested. */
   if (latest_rev)
     SVN_ERR ((*ra_lib)->get_latest_revnum (*session, latest_rev, pool));
@@ -486,7 +531,6 @@
 svn_client_import (svn_client_commit_info_t **commit_info,
                    const char *path,
                    const char *url,
-                   const char *new_entry,
                    svn_boolean_t nonrecursive,
                    svn_client_ctx_t *ctx,
                    apr_pool_t *pool)
@@ -501,20 +545,9 @@
   const char *committed_date = NULL;
   const char *committed_author = NULL;
   apr_hash_t *excludes = apr_hash_make (pool);
+  const char *new_entry = NULL;
 
-  /* Sanity check: NEW_ENTRY can be null or non-empty, but it can't be
-     empty. */
-  if (new_entry && (strcmp (new_entry, "") == 0))
-    return svn_error_create (SVN_ERR_FS_PATH_SYNTAX, NULL,
-                             "empty string is an invalid entry name");
 
-  /* The repository doesn't know about the reserved. */
-  if (new_entry && strcmp (new_entry, SVN_WC_ADM_DIR_NAME) == 0)
-    return svn_error_createf
-      (SVN_ERR_CL_ADM_DIR_RESERVED, NULL,
-       "the name \"%s\" is reserved and cannot be imported",
-       SVN_WC_ADM_DIR_NAME);
-
   /* Create a new commit item and add it to the array. */
   if (ctx->log_msg_func)
     {
@@ -550,18 +583,71 @@
     {
       svn_node_kind_t kind;
       const char *base_dir = path;
+      apr_array_header_t *dirs = NULL;
+      const char *temp;
+      const char *dir;
 
       SVN_ERR (svn_io_check_path (path, &kind, pool));
       if (kind == svn_node_file)
         svn_path_split (path, &base_dir, NULL, pool);
 
-      SVN_ERR (get_ra_editor (&ra_baton, &session, &ra_lib, NULL,
-                              &editor, &edit_baton, ctx, url, base_dir,
-                              NULL, log_msg, NULL, &committed_rev,
-                              &committed_date, &committed_author, 
-                              FALSE, pool));
+      err = NULL;
+
+      do
+        {
+          if (err)
+            {
+              /* If get_ra_editor below failed we either tried to open
+               * an invalid url, or else some other kind of error.  In case
+               * the url was bad we back up a directory and try again. */
+
+              if (err->apr_err != SVN_ERR_FS_NO_SUCH_ENTRY)
+                return err;
+
+              if (!dirs)
+                dirs = apr_array_make (pool, 1, sizeof(const char *));
+
+              svn_path_split (url, &temp, &dir, pool);
+              *((const char **)apr_array_push (dirs)) = dir;
+              url = temp;
+            }
+        }
+      while ((err = get_ra_editor (&ra_baton, &session, &ra_lib, NULL,
+                                   &editor, &edit_baton, ctx, url, base_dir,
+                                   NULL, log_msg, NULL, &committed_rev,
+                                   &committed_date, &committed_author,
+                                   FALSE, pool)));
+
+      if (dirs)
+        {
+          const char **child;
+
+          if ((child = (const char **) apr_array_pop(dirs)))
+            {
+              new_entry = *child;
+
+              while ((child = (const char **) apr_array_pop(dirs)))
+                new_entry = svn_path_join (new_entry, *child, pool);
+            }
+        }
+
+      /* new_entry == NULL means the first call to get_ra_editor
+       * above succeeded.  That means that 'url' corresponds to an
+       * already existing filesystem entity. */
+      if (kind == svn_node_file && ! new_entry)
+        return svn_error_createf
+          (SVN_ERR_ENTRY_EXISTS, NULL,
+           "the path \"%s\" already exists", url);
     }
 
+  /* The repository doesn't know about the reserved. */
+  if (new_entry && strcmp (new_entry, SVN_WC_ADM_DIR_NAME) == 0)
+    return svn_error_createf
+      (SVN_ERR_CL_ADM_DIR_RESERVED, NULL,
+       "the name \"%s\" is reserved and cannot be imported",
+       SVN_WC_ADM_DIR_NAME);
+
+
   /* If an error occured during the commit, abort the edit and return
      the error.  We don't even care if the abort itself fails.  */
   if ((err = import (path, new_entry,
Index: subversion/clients/cmdline/man/svn.1
===================================================================
--- subversion/clients/cmdline/man/svn.1	(revision 6178)
+++ subversion/clients/cmdline/man/svn.1	(working copy)
@@ -220,7 +220,7 @@
 command is entered, a short description on how to use that command is
 presented.
 .TP
-\fBimport\fP \fIRepository-URL\fP [\fIPath\fP] [\fINew-Repository-Entry\fP] [\fI--username name\fP] [\fI--password pass\fP] [\fI--no-auth-cache\fP] [\fI--encoding\fP] [\fI-FmqN\fP]
+\fBimport\fP [\fIPath\fP] \fIRepository-URL\fP [\fI--username name\fP] [\fI--password pass\fP] [\fI--no-auth-cache\fP] [\fI--encoding\fP] [\fI-FmqN\fP]
 Import a file or tree into the repository.
 .TP
 \fBinfo\fP \fItarget1\fP [\fItarget2\fP ...] [\fI--targets file\fP] [\fI-R\fP]
Index: subversion/clients/cmdline/import-cmd.c
===================================================================
--- subversion/clients/cmdline/import-cmd.c	(revision 6178)
+++ subversion/clients/cmdline/import-cmd.c	(working copy)
@@ -44,36 +44,33 @@
   apr_array_header_t *targets;
   const char *path;
   const char *url;
-  const char *new_entry;
   svn_client_commit_info_t *commit_info = NULL;
 
-  /* Import takes up to three arguments, for example
+  /* Import takes two arguments, for example
    *
-   *   $ svn import  file:///home/jrandom/repos  ./myproj  myproj
-   *                 ^^^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^  ^^^^^^
-   *                        (repository)          (source)  (dest)
+   *   $ svn import projects/test file:///home/jrandom/repos/trunk
+   *                ^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   *                 (source)       (repository)
    *
    * or
    *
-   *   $ svn import  file:///home/jrandom/repos/some/subdir  .  myproj
+   *   $ svn import file:///home/jrandom/repos/some/subdir
    *
    * What is the nicest behavior for import, from the user's point of
    * view?  This is a subtle question.  Seemingly intuitive answers
    * can lead to weird situations, such never being able to create
    * non-directories in the top-level of the repository.
    *
-   * For now, let's keep things simple:
+   * If 'source' is a file then the basename of 'url' is used as the
+   * filename in the repository.  If 'source' is a directory then the
+   * import happens directly in the repository target dir, creating
+   * however many new entries are necessary.  If some part of 'url'
+   * does not exist in the repository then parent directories are created
+   * as necessary.
    *
-   * If the third arg is present, it is the name of the new entry in
-   * the repository target dir (the latter may or may not be the root
-   * dir).  If it is absent, then the import happens directly in the
-   * repository target dir, creating however many new entries are
-   * necessary.
+   * In the case where no 'source' is given '.' (the current directory)
+   * is implied.
    *
-   * If the second arg is also omitted, then "." is implied.
-   *
-   * The first arg cannot be omitted, of course.
-   *
    * ### kff todo: review above behaviors.
    */
 
@@ -83,31 +80,30 @@
                                          &(opt_state->end_revision),
                                          FALSE, pool));
 
-  /* Get a repository url. */
   if (targets->nelts < 1)
     return svn_error_create
       (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
        "repository url required when importing");
-  else
-    url = ((const char **) (targets->elts))[0];
-
-  /* Get a local path. */
-  if (targets->nelts < 2)
-    path = "";
-  else
-    path = ((const char **) (targets->elts))[1];
-
-  /* Optionally get the dest entry name. */
-  if (targets->nelts < 3)
-    new_entry = NULL;  /* tells import() to create many entries at top
-                          level. */
-  else if (targets->nelts == 3)
-    new_entry = ((const char **) (targets->elts))[2];
-  else
+  else if (targets->nelts > 2)
     return svn_error_create
       (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
        "too many arguments to import command");
-  
+  else if (targets->nelts == 1)
+    {
+      url = ((const char **) (targets->elts))[0];
+      path = "";
+    }
+  else
+    {
+      path = ((const char **) (targets->elts))[0];
+      url = ((const char **) (targets->elts))[1];
+    }
+
+  if (! svn_path_is_url (url))
+    return svn_error_createf
+      (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+       "invalid url '%s'", url);
+
   if (! opt_state->quiet)
     svn_cl__get_notifier (&ctx->notify_func, &ctx->notify_baton,
                           FALSE, FALSE, FALSE, pool);
@@ -118,7 +114,6 @@
            (ctx->log_msg_baton, svn_client_import (&commit_info,
                                                    path,
                                                    url,
-                                                   new_entry,
                                                    opt_state->nonrecursive,
                                                    ctx,
                                                    pool)));
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c	(revision 6178)
+++ subversion/clients/cmdline/main.c	(working copy)
@@ -248,11 +248,10 @@
   
   { "import", svn_cl__import, {0},
     "Commit an unversioned file or tree into the repository.\n"
-    "usage: import URL [PATH [NEW_ENTRY_IN_REPOS]]\n\n"
+    "usage: import [PATH] URL\n\n"
     "  Recursively commit a copy of PATH to URL.\n"
-    "  If no third arg, copy top-level contents of PATH into URL\n"
-    "  directly.  Otherwise, create NEW_ENTRY underneath URL and\n"
-    "  begin copy there.\n",
+    "  If PATH is omitted '.' is assumed.  Parent directories are created\n"
+    "  as necessary in the repository.\n",
     {'m', 'F', 'q', 'N', SVN_CL__AUTH_OPTIONS,
      svn_cl__editor_cmd_opt, svn_cl__encoding_opt} },
  
Index: subversion/tests/clients/cmdline/basic_tests.py
===================================================================
--- subversion/tests/clients/cmdline/basic_tests.py	(revision 6178)
+++ subversion/tests/clients/cmdline/basic_tests.py	(working copy)
@@ -1143,12 +1143,12 @@
   svntest.main.file_append(new_path, "some text")
 
   # import new files into repository
-  url = svntest.main.current_repo_url
+  url = os.path.join(svntest.main.current_repo_url, "dirA/dirB/new_file")
   output, errput =   svntest.actions.run_and_verify_svn(
     'Cannot change node kind', None, [], 'import',
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
-    '-m', 'Log message for new import', url, new_path, 'new_file')
+    '-m', 'Log message for new import', new_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
@@ -1163,19 +1163,23 @@
   # Create expected disk tree for the update (disregarding props)
   expected_disk = svntest.main.greek_state.copy()
   expected_disk.add({
-    'new_file' : Item('some text'),
+    'dirA/dirB/new_file' : Item('some text'),
     })
 
   # Create expected status tree for the update (disregarding props).
   # Newly imported file should be at revision 2.
   expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
   expected_status.add({
-    'new_file' : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA'                : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA/dirB'           : Item(status='  ', wc_rev=2, repos_rev=2),
+    'dirA/dirB//new_file' : Item(status='  ', wc_rev=2, repos_rev=2),
     })
 
   # Create expected output tree for the update.
   expected_output = svntest.wc.State(wc_dir, {
-    'new_file' : Item(status='A '),
+    'dirA'               : Item(status='A '),
+    'dirA/dirB'          : Item(status='A '),
+    'dirA/dirB/new_file' : Item(status='A '),
   })
 
   # do update and check three ways
@@ -1221,7 +1225,7 @@
     None, None, [], 'import',
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
-    '-m', 'Log message for new import', url, xt_path)
+    '-m', 'Log message for new import', xt_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
@@ -1483,7 +1487,7 @@
   open(foo_o_path, 'w')
 
   # import new dir into repository
-  url = svntest.main.current_repo_url
+  url = os.path.join(svntest.main.current_repo_url, 'dir')
 
 
   output, errput = svntest.actions.run_and_verify_svn(
@@ -1491,7 +1495,7 @@
     '--username', svntest.main.wc_author,
     '--password', svntest.main.wc_passwd,
     '-m', 'Log message for new import',
-    url, dir_path, 'dir')
+    dir_path, url)
 
   lastline = string.strip(output.pop())
   cm = re.compile ("(Committed|Imported) revision [0-9]+.")
Index: subversion/tests/clients/cmdline/svntest/actions.py
===================================================================
--- subversion/tests/clients/cmdline/svntest/actions.py	(revision 6178)
+++ subversion/tests/clients/cmdline/svntest/actions.py	(working copy)
@@ -78,7 +78,7 @@
                                   '--username', main.wc_author,
                                   '--password', main.wc_passwd,
                                   '-m', 'Log message for revision 1.',
-                                  url, main.greek_dump_dir)
+                                  main.greek_dump_dir, url)
 
     # check for any errors from the import
     if len(errput):
Index: doc/book/book/ch03.xml
===================================================================
--- doc/book/book/ch03.xml	(revision 6178)
+++ doc/book/book/ch03.xml	(working copy)
@@ -1972,11 +1972,9 @@
       <para>The import command is a quick way to move an unversioned
         tree of files into a repository.</para>
 
-      <para>There are two ways to use this command:</para>
-
       <screen>
 $ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree
+$ svn import mytree file:///usr/local/svn/newrepos/fooproject
 Adding  mytree/foo.c
 Adding  mytree/bar.c
 Adding  mytree/subdir
@@ -1986,34 +1984,10 @@
       </screen>
 
       <para>The above example places the contents of directory
-        <filename>mytree</filename> directly into the root of the
-        repository:</para>
+        <filename>mytree</filename> under the directory
+        <filename>fooproject</filename> in the repository:</para>
 
       <screen>
-/foo.c
-/bar.c
-/subdir
-/subdir/quux.h
-      </screen>
-
-      <para>If you give <command>svn import</command> a third
-        argument, it will use the argument as the name of a new
-        subdirectory to create within the URL.</para>
-
-      <screen>
-$ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree fooproject
-Adding  mytree/foo.c
-Adding  mytree/bar.c
-Adding  mytree/subdir
-Adding  mytree/subdir/quux.h
-Transmitting file data....
-Committed revision 1.
-      </screen>
-
-      <para>The repository would now look like this:</para>
-
-      <screen>
 /fooproject/foo.c
 /fooproject/bar.c
 /fooproject/subdir
Index: doc/book/book/ch05.xml
===================================================================
--- doc/book/book/ch05.xml	(revision 6178)
+++ doc/book/book/ch05.xml	(working copy)
@@ -2654,7 +2654,7 @@
 $ mkdir projectB/trunk
 $ mkdir projectB/branches
 &hellip;
-$ svn import file:///path/to/repos . --message 'Initial repository layout'
+$ svn import file:///path/to/repos --message 'Initial repository layout'
 Adding         projectA
 Adding         projectA/trunk
 Adding         projectA/branches
Index: doc/book/book/ch06.xml
===================================================================
--- doc/book/book/ch06.xml	(revision 6178)
+++ doc/book/book/ch06.xml	(working copy)
@@ -1679,13 +1679,9 @@
         first vendor drop.</para>
 
       <screen>
-$ svn mkdir http://svn.example.com/repos/calc/vendor/libcomplex  \
-            -m 'creating vendor branch area'
-
 &hellip;
-$ svn import http://svn.example.com/repos/calc/vendor/libcomplex \
-             /path/to/libcomplex-1.0                             \
-             current                                             \
+$ svn import /path/to/libcomplex-1.0 \
+             http://svn.example.com/repos/calc/vendor/libcomplex/current \
              -m 'importing initial 1.0 vendor drop'
 &hellip;
 </screen>
Index: doc/book/book/ch08.xml
===================================================================
--- doc/book/book/ch08.xml	(revision 6178)
+++ doc/book/book/ch08.xml	(working copy)
@@ -1306,18 +1306,15 @@
           </refnamediv>
           <refsect1>
             <title>Synopsis</title>
-            <programlisting>svn import URL [PATH [NEW_ENTRY_IN_REPOS]]</programlisting>
+            <programlisting>svn import [PATH] URL</programlisting>
           </refsect1>
           <refsect1>
             <title>Description</title>
 
             <para>Recursively commit a copy of <literal>PATH</literal>
-              to <literal>URL</literal>.  If there is no third
-              argument, copy the top-level contents of
-              <literal>PATH</literal> into <literal>URL</literal>
-              directly.  Otherwise, create
-              <literal>NEW_ENTRY</literal> underneath
-              <literal>URL</literal> and begin copy there.</para>
+              to <literal>URL</literal>.  If <literal>PATH</literal>
+              is omitted '.' is assumed.  Parent directories are created
+              in the repository as necessary.</para>
           </refsect1>
 
           <refsect1>
@@ -1358,7 +1355,7 @@
               root of your repository:</para>
 
             <screen>
-$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj
+$ svn import -m "New import" myproj http://svn.red-bean.com/repos/test
 Adding         myproj/sample.txt
 ...
 Transmitting file data .........
@@ -1367,12 +1364,11 @@
 
             <para>This imports the local directory 'myproj' into
               'trunk/vendors' in your repository.  The directory
-              'trunk/vendors' must exist before you import into
-              it&mdash;<command>svn import</command> will not
+              'trunk/vendors' need not exist before you import into
+              it&mdash;<command>svn import</command> will
               recursively create directories for you:</para>
-
             <screen>
-$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj trunk/vendors/myproj
+$ svn import -m "New import" myproj http://svn.red-bean.com/repos/test/trunk/vendors/myproj
 Adding         myproj/sample.txt
 ...
 Transmitting file data .........

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by Robert Pluim <rp...@bigfoot.com>.
David Kimdon writes:
 > On Mon, Jun 02, 2003 at 11:43:04AM +0200, Robert Pluim wrote:
 > >  > Parent directories in URL are created as necessary.  Testsuite and
 > >  > documentation updated as well.
 > > 
 > > Hmm, what happens if I do 'svn import foo/bar file://somewhere/repo/'?
 > > Do we end up with 'file://somewhere/repo/foo/bar'?
 > 
 > No.  URL is used to set the location in the repository.  PATH is used
 > to find the directory tree to import on the local (non-subversion)
 > filesystem.

OK.  Seems fine (and consistent) to me.

Robert

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by David Kimdon <dw...@debian.org>.
On Mon, Jun 02, 2003 at 11:43:04AM +0200, Robert Pluim wrote:
>  > Parent directories in URL are created as necessary.  Testsuite and
>  > documentation updated as well.
> 
> Hmm, what happens if I do 'svn import foo/bar file://somewhere/repo/'?
> Do we end up with 'file://somewhere/repo/foo/bar'?

No.  URL is used to set the location in the repository.  PATH is used
to find the directory tree to import on the local (non-subversion)
filesystem.

$ ls foo/bar
README
$

'svn import foo/bar file:///somewhere/repos' will create
'file:///somewhere/repos/README'

'svn import foo/bar file:///somewhere/repos/foo/bar' will create
'file:///somewhere/repos/foo/bar/README'

-David

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [rfc/patch] : #933,#735 import syntax, mkdir -p

Posted by Robert Pluim <rp...@bigfoot.com>.
David Kimdon writes:
 > Hi,
 > 
 > Here is a patch for the import issues indicated below.  Any comments
 > and/or criticism is, of course, welcome.  I'm particularly interested
 > in comments on the change I made to get_ra_editor().  I need something
 > like 'stat()' for the repository, and I believe that this extra error
 > checking in get_ra_editor() does no harm, but maybe I missed
 > something.  The change to subversion/libsvn_client/commit.c(import) is
 > similar to that which has been reviewed by cmpilato previously.
 > 
 > -David
 > 
 > Fix issue #933: svn import syntax is confusing; merge 1st and 3rd arguments
 > Fix issue #735: import should 'mkdir -p'
 > 
 > Import syntax is now:
 > 
 > svn import [PATH] URL
 >

I was going to complain about the switching of the PATH and URL
arguments, then I realised that 'import' now matches 'cp', which is
good.

+1

 > Parent directories in URL are created as necessary.  Testsuite and
 > documentation updated as well.

Hmm, what happens if I do 'svn import foo/bar file://somewhere/repo/'?
Do we end up with 'file://somewhere/repo/foo/bar'?

Robert

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org