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/05/02 04:11:40 UTC
[rfc/patch] Fix issue 734, import should 'mkdir -p'
Hi,
Here's a fix that I'd like another set of eyes on. It passes 'make
check', of course. I'm not entirely sure that I'm doing the right
thing closing the directory batons while I'm doing the mkdir's. I'm
keeping just the last one, and the root baton open and closing the
intermediate batons.
Thanks,
-David
Fix issue #735: Create directories on import.
* subversion/libsvn_client/commit.c (import) : Call mkdir on
components of 'new_entry' prior to performing the actual import.
* subversion/tests/clients/cmdline/basic_tests.py (basic_import) : Change
existing test case to use a path to the third argument to import and
verify that the new directories are created properly.
* doc/book/book/ch03.xml : Change text and import example to show how
subversion can create subdirectories on import.
* doc/book/book/ch08.xml : Remove warning that the leading elements of
the third argument to import must exist. Subversion will now create
these directories.
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 5780)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -341,6 +341,7 @@
void *root_baton;
svn_node_kind_t kind;
apr_array_header_t *ignores;
+ void *new_dir_baton = 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 +352,42 @@
/* Import a file or a directory tree. */
SVN_ERR (svn_io_check_path (path, &kind, pool));
+ if (new_entry) {
+ apr_array_header_t *temp, *dirs;
+ const char **element;
+ const char *new_path = "";
+
+ temp = svn_path_decompose(new_entry, pool);
+ dirs = apr_array_make (pool, 1, sizeof(const char *));
+
+ /* 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. Discard the component. */
+ if (kind == svn_node_file) {
+ apr_array_pop(temp);
+ }
+
+ /* We can only pop paths off the array, and they won't come
+ * off in the order we want to create directories. Create
+ * another array that we can access in the desired order. */
+ while ((element = (const char **) apr_array_pop(temp))) {
+ *((const char **) apr_array_push(dirs)) = *element;
+ }
+
+ while ((element = (const char **) apr_array_pop(dirs))) {
+
+ if (new_dir_baton)
+ SVN_ERR (editor->close_directory (new_dir_baton, pool));
+
+ new_path = svn_path_join(new_path, *element, pool);
+
+ SVN_ERR (editor->add_directory (new_path, root_baton,
+ NULL, SVN_INVALID_REVNUM,
+ pool, &new_dir_baton));
+ }
+ }
+
+
/* 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
@@ -368,20 +405,13 @@
(SVN_ERR_NODE_UNKNOWN_KIND, NULL,
"new entry name required when importing a file");
- SVN_ERR (import_file (editor, root_baton,
+ SVN_ERR (import_file (editor,
+ new_dir_baton ? new_dir_baton : root_baton,
path, new_entry, ctx, pool));
}
}
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
*
@@ -419,9 +449,6 @@
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)
{
@@ -431,6 +458,8 @@
}
/* Close up the show; it's time to go home. */
+ if (new_dir_baton)
+ SVN_ERR (editor->close_directory (new_dir_baton, pool));
SVN_ERR (editor->close_directory (root_baton, pool));
SVN_ERR (editor->close_edit (edit_baton, pool));
Index: subversion/tests/clients/cmdline/basic_tests.py
===================================================================
--- subversion/tests/clients/cmdline/basic_tests.py (revision 5780)
+++ subversion/tests/clients/cmdline/basic_tests.py (working copy)
@@ -1148,7 +1148,7 @@
'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', url, new_path, 'dir1/dir2/new_file')
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'),
+ os.path.join('dir1', 'dir2', '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),
+ 'dir1' : Item(status=' ', wc_rev=2, repos_rev=2),
+ 'dir1/dir2' : Item(status=' ', wc_rev=2, repos_rev=2),
+ 'dir1/dir2/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 '),
+ 'dir1' : Item(status='A '),
+ 'dir1/dir2' : Item(status='A '),
+ 'dir1/dir2/new_file' : Item(status='A '),
})
# do update and check three ways
Index: doc/book/book/ch03.xml
===================================================================
--- doc/book/book/ch03.xml (revision 5780)
+++ doc/book/book/ch03.xml (working copy)
@@ -1996,11 +1996,11 @@
<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>
+ path to create within the URL.</para>
<screen>
$ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree fooproject
+$ svn import file:///usr/local/svn/newrepos mytree fooproject/trunk
Adding mytree/foo.c
Adding mytree/bar.c
Adding mytree/subdir
@@ -2012,10 +2012,10 @@
<para>The repository would now look like this:</para>
<screen>
-/fooproject/foo.c
-/fooproject/bar.c
-/fooproject/subdir
-/fooproject/subdir/quux.h
+/fooproject/trunk/foo.c
+/fooproject/trunk/bar.c
+/fooproject/trunk/subdir
+/fooproject/trunk/subdir/quux.h
</screen>
</sect2>
Index: doc/book/book/ch08.xml
===================================================================
--- doc/book/book/ch08.xml (revision 5780)
+++ doc/book/book/ch08.xml (working copy)
@@ -1366,10 +1366,7 @@
</screen>
<para>This imports the local directory 'myproj' into
- 'trunk/vendors' in your repository. The directory
- 'trunk/vendors' must exist before you import into
- it—<command>svn import</command> will not
- recursively create directories for you:</para>
+ 'trunk/vendors' in your repository:</para>
<screen>
$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj trunk/vendors/myproj
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by cm...@collab.net.
David Kimdon <dw...@debian.org> writes:
> Sure, so how best to proceed? The two fixes are intertwined, but
> distinct. It is looking like 933 will make larger changes than 734
> and may also remove the very function that 734 touches. 933 is
> unassigned, I might take it on myself, since I'm in a bug fixing
> mood.
It would appear that this issue (#734) should die, and issue #933 be
the thing to tackle.
> In the last message to issue #933 Karl summarizes a thread [1] that
> indicates the new syntax should be :
>
> svn import [SRC] DST_URL
>
> Note that in addition to removing new_repos_entry, source and url are
> reversed. That makes import's syntax more like 'svn cp'. 'import'
> will be a special case of 'svn cp'. The new syntax will be similar to
> traditional UNIX cp, mv, rsync and cvs import syntax. Multiple
> sources will not be supported (at least not yet). SRC is optional, in
> which case '.' is implied.
>
> Any objections, suggestions, questions?
Well, I objected to the reversed ordering, but I was rather shot down
by my peers here at the Chicago CollabNet office. :-) So, in the
essence of not contributing to what is likely a bikeshed discussion,
I'd say just code the suggested interface.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by David Kimdon <dw...@debian.org>.
On Tue, May 06, 2003 at 11:41:29PM -0500, cmpilato@collab.net wrote:
> Jani Averbach <ja...@verso.st.jyu.fi> writes:
>
> > On 6 May 2003 cmpilato@collab.net wrote:
> >
> > > Am I alone in thinking that it's ridiculous that import takes 3
> > > arguments at all?
> >
> > Not. =) You could count at least me and Peter Davis in the same wagon:
> >
> > http://subversion.tigris.org/issues/show_bug.cgi?id=933
>
> Oh, well, there you have it.
Sure, so how best to proceed? The two fixes are intertwined, but
distinct. It is looking like 933 will make larger changes than 734
and may also remove the very function that 734 touches. 933 is
unassigned, I might take it on myself, since I'm in a bug fixing mood.
> > > I mean, in my ideal world, import would look like:
> > >
> > > svn import DESTINATION SOURCE [...]
> > >
> > > If destination didn't exist, it and all the intermediate directories
> > > needed would be created. If there were multiple SOURCE(s), then
> > > DESTINATION would be created as a directory, and each of the sources
> > > added to it by their basenames.
> >
> > The issue suggested that source is optional, but I like it to be
> > mandatory. It is too easy otherwise to import something that wasn't your
> > intention.
>
> I agree with this. I'd prefer SOURCE to be required.
In the last message to issue #933 Karl summarizes a thread [1] that
indicates the new syntax should be :
svn import [SRC] DST_URL
Note that in addition to removing new_repos_entry, source and url are
reversed. That makes import's syntax more like 'svn cp'. 'import'
will be a special case of 'svn cp'. The new syntax will be similar to
traditional UNIX cp, mv, rsync and cvs import syntax. Multiple
sources will not be supported (at least not yet). SRC is optional, in
which case '.' is implied.
Any objections, suggestions, questions?
-David
[1] : I don't mean to start a discussion on the topic unless someone
has something to add in addition to the good discussion that starts
here:
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=23205
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by Sander Striker <st...@apache.org>.
> From: cmpilato@collab.net [mailto:cmpilato@collab.net]
> Sent: Wednesday, May 07, 2003 6:41 AM
[...]
>> http://subversion.tigris.org/issues/show_bug.cgi?id=933
> Oh, well, there you have it.
>
>>> I mean, in my ideal world, import would look like:
>>>
>>> svn import DESTINATION SOURCE [...]
>>>
>>> If destination didn't exist, it and all the intermediate directories
>>> needed would be created. If there were multiple SOURCE(s), then
>>> DESTINATION would be created as a directory, and each of the sources
>>> added to it by their basenames.
>>
>> The issue suggested that source is optional, but I like it to be
>> mandatory. It is too easy otherwise to import something that wasn't your
>> intention.
>
> I agree with this. I'd prefer SOURCE to be required.
+1.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by cm...@collab.net.
Jani Averbach <ja...@verso.st.jyu.fi> writes:
> On 6 May 2003 cmpilato@collab.net wrote:
>
> > Am I alone in thinking that it's ridiculous that import takes 3
> > arguments at all?
>
> Not. =) You could count at least me and Peter Davis in the same wagon:
>
> http://subversion.tigris.org/issues/show_bug.cgi?id=933
Oh, well, there you have it.
> > I mean, in my ideal world, import would look like:
> >
> > svn import DESTINATION SOURCE [...]
> >
> > If destination didn't exist, it and all the intermediate directories
> > needed would be created. If there were multiple SOURCE(s), then
> > DESTINATION would be created as a directory, and each of the sources
> > added to it by their basenames.
>
> The issue suggested that source is optional, but I like it to be
> mandatory. It is too easy otherwise to import something that wasn't your
> intention.
I agree with this. I'd prefer SOURCE to be required.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by Jani Averbach <ja...@verso.st.jyu.fi>.
On 6 May 2003 cmpilato@collab.net wrote:
> Am I alone in thinking that it's ridiculous that import takes 3
> arguments at all?
Not. =) You could count at least me and Peter Davis in the same wagon:
http://subversion.tigris.org/issues/show_bug.cgi?id=933
> I mean, in my ideal world, import would look like:
>
> svn import DESTINATION SOURCE [...]
>
> If destination didn't exist, it and all the intermediate directories
> needed would be created. If there were multiple SOURCE(s), then
> DESTINATION would be created as a directory, and each of the sources
> added to it by their basenames.
The issue suggested that source is optional, but I like it to be
mandatory. It is too easy otherwise to import something that wasn't your
intention.
Here is a long thread about the topic from the past:
http://www.contactor.se/~dast/svn/archive-2002-10/0746.shtml
BR, Jani
--
Jani Averbach
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by cm...@collab.net.
David Kimdon <dw...@debian.org> writes:
> On Sat, May 03, 2003 at 11:15:27AM -0500, cmpilato@collab.net wrote:
> > Actually, the patch needs to be in the correct style before I exert
> > any energy on it. David, if you can whip this sucker into style-wise,
> > I'll give it a look-see implementation-wise.
>
> oops, sorry about that, here's try #3.
>
> -David
>
> Fix issue #735: Create directories on import.
>
> * subversion/libsvn_client/commit.c (import) : Call mkdir on
> components of 'new_entry' prior to performing the actual import.
>
> * subversion/tests/clients/cmdline/basic_tests.py (basic_import) : Change
> existing test case to use a path to the third argument to import and
> verify that the new directories are created properly.
>
> * doc/book/book/ch03.xml : Change text and import example to show how
> subversion can create subdirectories on import.
>
> * doc/book/book/ch08.xml : Remove warning that the leading elements of
> the third argument to import must exist. Subversion will now create
> these directories.
I've reviewed this patch, and generally it looked pretty good. Only
a couple of obvious problems I saw:
> + for (i = 0; i < dirs->nelts; i++)
> + {
> + if (new_dir_baton)
> + {
> + if (!batons)
> + {
> + batons = apr_array_make (pool, 1, sizeof (void *));
> + }
> +
> + *((void **) apr_array_push (batons)) = new_dir_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, &new_dir_baton));
> + }
In this loop, you're continually calling add_directory with a parent
dir baton of root_baton, instead of using the dir_baton you got from
the previous iteration.
> /* Close up the show; it's time to go home. */
> + if (new_dir_baton)
> + SVN_ERR (editor->close_directory (new_dir_baton, pool));
> +
> + {
> + void **baton;
> +
> + while ((baton = (void **) apr_array_pop (batons)))
> + {
> + SVN_ERR (editor->close_directory (*baton, pool));
> + }
> + }
> +
Here, you dive into the array without making sure it's non-NULL.
--
Aside from those things, it all looks pretty good. I've corrected the
above problems on my local machine, and am gonna do a little
hand-testing to verify that *I* didn't break something. After that,
I'll commit crediting you with the patch.
Of course, there is one other thing bugging me. It's not about your
patch, it's about the original issue.
Am I alone in thinking that it's ridiculous that import takes 3
arguments at all? I mean, in my ideal world, import would look like:
svn import DESTINATION SOURCE [...]
If destination didn't exist, it and all the intermediate directories
needed would be created. If there were multiple SOURCE(s), then
DESTINATION would be created as a directory, and each of the sources
added to it by their basenames.
I'd like to argue this design before your patch goes in, if that's
alright with you.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by David Kimdon <dw...@debian.org>.
On Sat, May 03, 2003 at 11:15:27AM -0500, cmpilato@collab.net wrote:
> Actually, the patch needs to be in the correct style before I exert
> any energy on it. David, if you can whip this sucker into style-wise,
> I'll give it a look-see implementation-wise.
oops, sorry about that, here's try #3.
-David
Fix issue #735: Create directories on import.
* subversion/libsvn_client/commit.c (import) : Call mkdir on
components of 'new_entry' prior to performing the actual import.
* subversion/tests/clients/cmdline/basic_tests.py (basic_import) : Change
existing test case to use a path to the third argument to import and
verify that the new directories are created properly.
* doc/book/book/ch03.xml : Change text and import example to show how
subversion can create subdirectories on import.
* doc/book/book/ch08.xml : Remove warning that the leading elements of
the third argument to import must exist. Subversion will now create
these directories.
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 5790)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -341,6 +341,8 @@
void *root_baton;
svn_node_kind_t kind;
apr_array_header_t *ignores;
+ apr_array_header_t *batons = NULL;
+ void *new_dir_baton = 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,44 @@
/* 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.) Discard the component. */
+ if (kind == svn_node_file)
+ {
+ apr_array_pop (dirs);
+ }
+
+ for (i = 0; i < dirs->nelts; i++)
+ {
+ if (new_dir_baton)
+ {
+ if (!batons)
+ {
+ batons = apr_array_make (pool, 1, sizeof (void *));
+ }
+
+ *((void **) apr_array_push (batons)) = new_dir_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, &new_dir_baton));
+ }
+ }
+
/* 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
@@ -368,20 +408,13 @@
(SVN_ERR_NODE_UNKNOWN_KIND, NULL,
"new entry name required when importing a file");
- SVN_ERR (import_file (editor, root_baton,
+ SVN_ERR (import_file (editor,
+ new_dir_baton ? new_dir_baton : root_baton,
path, new_entry, ctx, pool));
}
}
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
*
@@ -419,9 +452,6 @@
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)
{
@@ -431,6 +461,18 @@
}
/* Close up the show; it's time to go home. */
+ if (new_dir_baton)
+ SVN_ERR (editor->close_directory (new_dir_baton, pool));
+
+ {
+ void **baton;
+
+ while ((baton = (void **) apr_array_pop (batons)))
+ {
+ SVN_ERR (editor->close_directory (*baton, pool));
+ }
+ }
+
SVN_ERR (editor->close_directory (root_baton, pool));
SVN_ERR (editor->close_edit (edit_baton, pool));
Index: subversion/tests/clients/cmdline/basic_tests.py
===================================================================
--- subversion/tests/clients/cmdline/basic_tests.py (revision 5790)
+++ subversion/tests/clients/cmdline/basic_tests.py (working copy)
@@ -1148,7 +1148,7 @@
'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', url, new_path, 'dir1/dir2/new_file')
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'),
+ os.path.join('dir1', 'dir2', '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),
+ 'dir1' : Item(status=' ', wc_rev=2, repos_rev=2),
+ 'dir1/dir2' : Item(status=' ', wc_rev=2, repos_rev=2),
+ 'dir1/dir2/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 '),
+ 'dir1' : Item(status='A '),
+ 'dir1/dir2' : Item(status='A '),
+ 'dir1/dir2/new_file' : Item(status='A '),
})
# do update and check three ways
Index: doc/book/book/ch03.xml
===================================================================
--- doc/book/book/ch03.xml (revision 5790)
+++ doc/book/book/ch03.xml (working copy)
@@ -1996,11 +1996,11 @@
<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>
+ path to create within the URL.</para>
<screen>
$ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree fooproject
+$ svn import file:///usr/local/svn/newrepos mytree fooproject/trunk
Adding mytree/foo.c
Adding mytree/bar.c
Adding mytree/subdir
@@ -2012,10 +2012,10 @@
<para>The repository would now look like this:</para>
<screen>
-/fooproject/foo.c
-/fooproject/bar.c
-/fooproject/subdir
-/fooproject/subdir/quux.h
+/fooproject/trunk/foo.c
+/fooproject/trunk/bar.c
+/fooproject/trunk/subdir
+/fooproject/trunk/subdir/quux.h
</screen>
</sect2>
Index: doc/book/book/ch08.xml
===================================================================
--- doc/book/book/ch08.xml (revision 5790)
+++ doc/book/book/ch08.xml (working copy)
@@ -1366,10 +1366,7 @@
</screen>
<para>This imports the local directory 'myproj' into
- 'trunk/vendors' in your repository. The directory
- 'trunk/vendors' must exist before you import into
- it—<command>svn import</command> will not
- recursively create directories for you:</para>
+ 'trunk/vendors' in your repository:</para>
<screen>
$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj trunk/vendors/myproj
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by cm...@collab.net.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> On Saturday, May 3, 2003, at 11:50 AM, David Kimdon wrote:
>
> > On Fri, May 02, 2003 at 07:47:09AM -0500, cmpilato@collab.net wrote:
> >> Unfortunately, that's not quite right. The rules of editor driving
> >> require that you keep all the intermediate directories open, closing
> >
> > k, thanks for the tip, here is an update. Do we like it?
>
> i'll let cmpilato comment on the implementation itself, but one thing
> i noticed...
Actually, the patch needs to be in the correct style before I exert
any energy on it. David, if you can whip this sucker into style-wise,
I'll give it a look-see implementation-wise.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Saturday, May 3, 2003, at 11:50 AM, David Kimdon wrote:
> On Fri, May 02, 2003 at 07:47:09AM -0500, cmpilato@collab.net wrote:
>> Unfortunately, that's not quite right. The rules of editor driving
>> require that you keep all the intermediate directories open, closing
>
> k, thanks for the tip, here is an update. Do we like it?
i'll let cmpilato comment on the implementation itself, but one thing i
noticed...
> + 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.) Discard the component. */
> + if (kind == svn_node_file) {
> + apr_array_pop (dirs);
> + }
> +
> + for (i = 0; i < dirs->nelts; i++) {
> +
> + if (new_dir_baton) {
> +
> + if (!batons) {
> + batons = apr_array_make (pool, 1, sizeof (void *));
> + }
> +
> + *((void **) apr_array_push (batons)) = new_dir_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, &new_dir_baton));
> + }
> + }
the indentation here (and in the rest of the patch) is kind of off. we
use the gnu style, so it should be something like:
for (i = 0; i < blargh; ++i)
{
if (blah)
{
foo ();
}
else
{
bar ();
}
}
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by David Kimdon <dw...@debian.org>.
On Fri, May 02, 2003 at 07:47:09AM -0500, cmpilato@collab.net wrote:
> Unfortunately, that's not quite right. The rules of editor driving
> require that you keep all the intermediate directories open, closing
k, thanks for the tip, here is an update. Do we like it?
-David
Fix issue #735: Create directories on import.
* subversion/libsvn_client/commit.c (import) : Call mkdir on
components of 'new_entry' prior to performing the actual import.
* subversion/tests/clients/cmdline/basic_tests.py (basic_import) : Change
existing test case to use a path to the third argument to import and
verify that the new directories are created properly.
* doc/book/book/ch03.xml : Change text and import example to show how
subversion can create subdirectories on import.
* doc/book/book/ch08.xml : Remove warning that the leading elements of
the third argument to import must exist. Subversion will now create
these directories.
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 5790)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -341,6 +341,8 @@
void *root_baton;
svn_node_kind_t kind;
apr_array_header_t *ignores;
+ apr_array_header_t *batons = NULL;
+ void *new_dir_baton = 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.) Discard the component. */
+ if (kind == svn_node_file) {
+ apr_array_pop (dirs);
+ }
+
+ for (i = 0; i < dirs->nelts; i++) {
+
+ if (new_dir_baton) {
+
+ if (!batons) {
+ batons = apr_array_make (pool, 1, sizeof (void *));
+ }
+
+ *((void **) apr_array_push (batons)) = new_dir_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, &new_dir_baton));
+ }
+ }
+
/* 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
@@ -368,20 +404,13 @@
(SVN_ERR_NODE_UNKNOWN_KIND, NULL,
"new entry name required when importing a file");
- SVN_ERR (import_file (editor, root_baton,
+ SVN_ERR (import_file (editor,
+ new_dir_baton ? new_dir_baton : root_baton,
path, new_entry, ctx, pool));
}
}
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
*
@@ -419,9 +448,6 @@
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)
{
@@ -431,6 +457,17 @@
}
/* Close up the show; it's time to go home. */
+ if (new_dir_baton)
+ SVN_ERR (editor->close_directory (new_dir_baton, pool));
+
+ {
+ void **baton;
+
+ while ((baton = (void **) apr_array_pop (batons))) {
+ SVN_ERR (editor->close_directory (*baton, pool));
+ }
+ }
+
SVN_ERR (editor->close_directory (root_baton, pool));
SVN_ERR (editor->close_edit (edit_baton, pool));
Index: subversion/tests/clients/cmdline/basic_tests.py
===================================================================
--- subversion/tests/clients/cmdline/basic_tests.py (revision 5790)
+++ subversion/tests/clients/cmdline/basic_tests.py (working copy)
@@ -1148,7 +1148,7 @@
'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', url, new_path, 'dir1/dir2/new_file')
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'),
+ os.path.join('dir1', 'dir2', '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),
+ 'dir1' : Item(status=' ', wc_rev=2, repos_rev=2),
+ 'dir1/dir2' : Item(status=' ', wc_rev=2, repos_rev=2),
+ 'dir1/dir2/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 '),
+ 'dir1' : Item(status='A '),
+ 'dir1/dir2' : Item(status='A '),
+ 'dir1/dir2/new_file' : Item(status='A '),
})
# do update and check three ways
Index: doc/book/book/ch03.xml
===================================================================
--- doc/book/book/ch03.xml (revision 5790)
+++ doc/book/book/ch03.xml (working copy)
@@ -1996,11 +1996,11 @@
<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>
+ path to create within the URL.</para>
<screen>
$ svnadmin create /usr/local/svn/newrepos
-$ svn import file:///usr/local/svn/newrepos mytree fooproject
+$ svn import file:///usr/local/svn/newrepos mytree fooproject/trunk
Adding mytree/foo.c
Adding mytree/bar.c
Adding mytree/subdir
@@ -2012,10 +2012,10 @@
<para>The repository would now look like this:</para>
<screen>
-/fooproject/foo.c
-/fooproject/bar.c
-/fooproject/subdir
-/fooproject/subdir/quux.h
+/fooproject/trunk/foo.c
+/fooproject/trunk/bar.c
+/fooproject/trunk/subdir
+/fooproject/trunk/subdir/quux.h
</screen>
</sect2>
Index: doc/book/book/ch08.xml
===================================================================
--- doc/book/book/ch08.xml (revision 5790)
+++ doc/book/book/ch08.xml (working copy)
@@ -1366,10 +1366,7 @@
</screen>
<para>This imports the local directory 'myproj' into
- 'trunk/vendors' in your repository. The directory
- 'trunk/vendors' must exist before you import into
- it—<command>svn import</command> will not
- recursively create directories for you:</para>
+ 'trunk/vendors' in your repository:</para>
<screen>
$ svn import -m "New import" http://svn.red-bean.com/repos/test myproj trunk/vendors/myproj
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [rfc/patch] Fix issue 734, import should 'mkdir -p'
Posted by cm...@collab.net.
David Kimdon <dw...@debian.org> writes:
> Hi,
>
> Here's a fix that I'd like another set of eyes on. It passes 'make
> check', of course. I'm not entirely sure that I'm doing the right
> thing closing the directory batons while I'm doing the mkdir's. I'm
> keeping just the last one, and the root baton open and closing the
> intermediate batons.
Unfortunately, that's not quite right. The rules of editor driving
require that you keep all the intermediate directories open, closing
them only as come back from the recursion on your way to closing the
root baton. A simply apr_array_header_t * of dir_batons should do the
trick for you. As an example, I did something very similar when I wrote
libsvn_client/copy.c:repos_to_repos_copy().
But thanks for working on this!
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org