Hello
Alexander Sinyushkin asked in IRC if it's a bug that export does not fail
for nonexistent URLs and DannyB said that many more people reported that
bug. So I wrote the following patch.
I ran the export tests only which show the following output before the
patch to export.c:
No error where one is expected
EXCEPTIN: SVNExpectedStderr
FAIL: export_tests.py 3: export a nonexistent url
And this with the patch:
PASS: export_tests.py 2: export the greek tree
PASS: export_tests.py 3: export a nonexistent url
PASS: export_tests.py 4: export working copy
Shall I run the full test suite or is it enough to run the export tests
only for such a trivial patch?
If noone objects I'm going to run the full test suite and commit this in a
few days.
Martin
[[[
Print error on export of nonexistent URL.
*
(svn_client_export3): Return error for nonexistent URL.
*
(export_nonexistent_url): New function.
(test_list): Add new test.
Suggested by: Alexander Sinyushkin <Alexander.Sinyushkin (AT) svnkit (DOT) com>
Approved by:
]]]
Index:
(revision 23186)
(working copy)
@@ -889,6 +889,9 @@
SVN_ERR(svn_client__fetch_externals(eb->externals, TRUE,
&use_sleep, ctx, pool));
}
+ else
+ return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
+ _("URL '%s' doesn't exist"), from);
}
else
{
Index:
(revision 23186)
(working copy)
@@ -64,6 +64,17 @@
expected_output,
svntest.main.greek_state.copy())
+def export_nonexistent_url(sbox):
+ "export a nonexistent url"
+ sbox.build(create_wc = False)
+
+ svntest.main.safe_rmtree(sbox.wc_dir)
+ export_target = sbox.wc_dir
+ nonexistent_url = sbox.repo_url + '/A/C/nonexistent'
+ ("No error where one is expected",
+ None, svntest.SVNA,
+ 'export', nonexistent_url, export_target)
+
def export_working_copy(sbox):
"export working copy"
sbox.build()
@@ -393,6 +404,7 @@
test_list = [ None,
export_empty_directory,
export_greek_tree,
+ export_nonexistent_url,
export_working_copy,
export_working_copy_with_mods,
export_over_existing_dir,
To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

Martin Furter wrote:
Shall I run the full test suite or is it enough to run the export
tests only for such a trivial patch?
I typically run the test set that is most likely to be affected by the
code with ra_dav/ra_serf and let the BuildBot test the other
combinations post-commit. Since this is about URL handling I'd run
export_tests.py for the 3 combinations.
If noone objects I'm going to run the full test suite and commit this
in a few days.
Did someone already approve this patch?

Lieven

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

Tue, 23 Jan 2007, Lieven Govaerts wrote:

Martin Furter wrote:

If noone objects I'm going to run the full test suite and commit this
in a few days.

Did someone already approve this patch?

Not that I'm aware of. I committed it to trunk in r23191 and r23193.
(I was at first unsure of the export.c change because I mis-read it a
couple of times, bleh.)

PGP SIGNATURE
Version: GnuPG v1.4.6 (GNU/Linux)

LdrK3fJ7IkSMB4AH6sGS/t8=
=BkWF
PGP SIGNATURE

Tue, 23 Jan 2007, Daniel Rall wrote:

Tue, 23 Jan 2007, Lieven Govaerts wrote:
>
>Martin Furter wrote:

If noone objects I'm going to run the full test suite and commit this
in a few days.
>>
>Did someone already approve this patch?

I thought it's an (almost) obvious fix but I don't dare to commit anything
I'm not 120% sure it's the right thing. I'm sorry I didn't want to scare
you :)

Not that I'm aware of. I committed it to trunk in r23191 and r23193.
(I was at first unsure of the export.c change because I mis-read it a
couple of times, bleh.)

Daniel, you changed my precious code! ;)
which causes it to fail here:

$ python export_tests.py 3
Error about nonexistent URL expected
EXCEPTIN: SVNExpectedStderr
FAIL: export_tests.py 3: attempt to export a nonexistent URL

Index:

(revision 23190)
(revision 23191)
@@ -64,6 +64,17 @@
expected_output,
svntest.main.greek_state.copy())

+def export_nonexistent_url(sbox):
+ "attempt to export a nonexistent URL"
+ sbox.build(create_wc = False)
I assume 'create_wc = False' means do not checkout so the sbox.wc_dir will
not exist.

+
+ svntest.main.safe_rmtree(sbox.wc_dir)
+ export_target = os.path.join(sbox.wc_dir, 'nonexistent')
If my assumption is right this join is not needed.

+ nonexistent_url = sbox.repo_url
sbox.repo_url is the base URL of the repos which always exists but we want
to checkout a *nonexisting* dir.

+ ("Error about nonexistent URL expected",
+ None, svntest.SVNA,
+ 'export', nonexistent_url, export_target)

Another question: Why did you add curly braces around the
'return svn_error_createf()' ?
I don't see anything in hacking or in the gnu standards document. I'm
just curious if this is the preferred way for new code in subversion so I
can do it better next time.

Martin

[[[
Fix the test for the fix of the attempt to export of a nonexistent URL.

*
(export_nonexistent_url): Make the URL point to a nonexistent node.
]]]

Index:

(revision 23199)
(working copy)
@@ -70,7 +70,7 @@

svntest.main.safe_rmtree(sbox.wc_dir)
export_target = os.path.join(sbox.wc_dir, 'nonexistent')
- nonexistent_url = sbox.repo_url
+ nonexistent_url = sbox.repo_url + "/nonexistent"
("Error about nonexistent URL expected",
None, svntest.SVNA,
'export', nonexistent_url, export_target)

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org

Wed, 24 Jan 2007, Martin Furter wrote:

Tue, 23 Jan 2007, Daniel Rall wrote:

Tue, 23 Jan 2007, Lieven Govaerts wrote:
>
>>Martin Furter wrote:
>
If noone objects I'm going to run the full test suite and commit this
in a few days.
>>
>>Did someone already approve this patch?

I thought it's an (almost) obvious fix but I don't dare to commit anything
I'm not 120% sure it's the right thing. I'm sorry I didn't want to scare
you :)

No problem. Lieven pointed that out because your commit access
doesn't extend to Subversion's core without approval from a full
committer. *shrug*

>Not that I'm aware of. I committed it to trunk in r23191 and r23193.
>(I was at first unsure of the export.c change because I mis-read it a
>couple of times, bleh.)

Daniel, you changed my precious code! ;)
which causes it to fail here:

$ python export_tests.py 3
Error about nonexistent URL expected
EXCEPTIN: SVNExpectedStderr
FAIL: export_tests.py 3: attempt to export a nonexistent URL

*sigh* Sorry Martin! I somehow managed to bungle commit of your patch
twice. :-(
Corrected on trunk, thanks for your patience.

Another question: Why did you add curly braces around the 'return
svn_error_createf()' ? I don't see anything in hacking or in the
gnu standards document. I'm just curious if this is the preferred
way for new code in subversion so I can do it better next time.

The braces aren't required by Subversion's coding conventions.
However, I found the code easier to read with the extra braces; the
difficulty I originally had reading the patch was caused by my eyes
and brain incorrectly assocating your "else" block with the previous
"if (! ignore_externals && recurse)" block.

Typically, when the above "if" or "else if" block uses braces, I use
braces on subsequent blocks so that things more easily match up
visually.
- Dan

PGP SIGNATURE
Version: GnuPG v1.4.6 (GNU/Linux)

7kkYsYKVkZUBKRwWWKIJGFQ=
=dBZh
PGP SIGNATURE

Tue, 23 Jan 2007, Daniel Rall wrote:

*sigh* Sorry Martin! I somehow managed to bungle commit of your patch
twice. :-(
Corrected on trunk, thanks for your patience.

No problem, for me the only important thing is that it's working now :)

Typically, when the above "if" or "else if" block uses braces, I use
braces on subsequent blocks so that things more easily match up
visually.

Yes, makes sense, I also find it more readable that way.

Thank you
Martin

To unsubscribe, e-mail: dev-unsubscribe (AT) subversion (DOT) tigris.org
For additional commands, e-mail: dev-help (AT) subversion (DOT) tigris.org