You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by John Szakmeister <jo...@szakmeister.net> on 2003/11/07 00:39:07 UTC
[PATCH] svn export needs tests (issue 506)
Here is a minor step in the right direction. :-) It adds a test for an export
of the entire greek tree.
Log:
---------------------------------
A step towards finishing issue #506. Adds a test of an exported greek tree.
* subversion/tests/clients/cmdline/export_tests.py
(export_greek_tree): New.
(test_list): Added new export_greek_tree test to the list.
* subversion/tests/clients/cmdline/svntest/actions.py
(run_and_verify_export): New.
---------------------------------
-John
[PATCH] svn export needs tests (issue 506) - more tests
Posted by John Szakmeister <jo...@szakmeister.net>.
I think this should probably be enough export tests to close out issue 506.
If I don't hear any objections, I'll commit the new tests later today. If
anyone can think of some other tests that should be here, let me know, I'm on
a roll! :-)
Log:
--------------------------------
Closes issue #506 (svn export needs tests). This adds 3 new export tests:
exporting a working copy, a working copy with local mods, and over an existing
directory.
* subversion/tests/clients/cmdline/export_tests.py
(export_working_copy, export_working_copy_with_mods,
export_over_existing_dir): New.
--------------------------------
-John
Re: [PATCH] svn export needs tests (issue 506)
Posted by John Szakmeister <jo...@szakmeister.net>.
On Saturday 08 November 2003 21:42, kfogel@collab.net wrote:
[snip]
> Those sound good; I can't think of anything else off the top of my
> head... Oh: maybe, exports that include eol translation and/or keyword
> substitution?
Good idea, I'll work that in with the other tests that I've done.
Thanks!
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 2: Modified export behavior
Posted by Erik Huelsmann <e....@gmx.net>.
> As for the keyword expansions: you can't count on the "current" user
> having any meaning in the repository username space. The system
> username might be totally unrelated to the usernames used by the
> repository. So $LastChangedBy$ should be unaffected, IMHO.
>
> As for the rev, if you want to put a "+", I think that's okay, but
> it's sort of a bikeshed to me... :-)
Keeping with svnversion convention the "+" should probably be "M"?
bye,
Erik.
--
NEU F�R ALLE - GMX MediaCenter - f�r Fotos, Musik, Dateien...
Fotoalbum, File Sharing, MMS, Multimedia-Gru�, GMX FotoService
Jetzt kostenlos anmelden unter http://www.gmx.net
+++ GMX - die erste Adresse f�r Mail, Message, More! +++
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 3: Modified export behavior
Posted by Julian Foad <ju...@btopenworld.com>.
John Szakmeister wrote:
> Thanks for the review Julian. Committed in r7813.
>
> Can I close issue 506 now that I've increased the export tests from 1 to 10?
I think you deserve to do so.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 3: Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
Thanks for the review Julian. Committed in r7813.
Can I close issue 506 now that I've increased the export tests from 1 to 10?
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 3: Modified export behavior
Posted by Julian Foad <ju...@btopenworld.com>.
John Szakmeister wrote:
> Okay, I think I've got all of this worked. I use the working copy timestamps
> only if the file is locally modified. For a locally modified file, I will
> also substitute in "(local)" as the author, append an 'M' to the revision
> number, and use the working copy time for LastChangedDate.
>
> So, here's another stab with everything. If no one disapproves of it, I'll
> commit it tomorrow, and hopefully move on to more serious problems. :-)
>
> -John
>
> Log:
> ----------------------------------------
> Fixes a minor issue with the 'export' command failing to copy the BASE
> revision of files when '-rBASE' is specified. Also fixes translation of
> keywords for locally modified files, as well as adding appropriate
> timestamping. Adds a couple of tests for this new functionality.
>
> * subversion/libsvn_client/export.c
> (copy_versioned_files): Added revision parameter. Changed the copy
> algorithm to translate keywords and eol style when appropriate.
>
> (svn_client_export): Added the ability to better distinguish when the
> repository should be contacted versus using the local base or working copy.
>
> * subversion/clients/cmdline/main.c
> (svn_cl__cmd_table): Updated to reflect export's new functionality.
>
> * subversion/tests/clients/cmdline/export_tests.py
> (export_working_copy_with_keyword_translation,
> export_working_copy_with_property_mods,
> export_working_copy_at_base_revision): New export tests.
>
> * subversion/tests/clients/cmdline/svntest/actions.py
> (run_and_verify_export): Added new *args parameter to support new export
> tests.
> ----------------------------------------
>
>
> Index: subversion/libsvn_client/export.c
> ===================================================================
> --- subversion/libsvn_client/export.c (revision 7778)
> +++ subversion/libsvn_client/export.c (working copy)
> @@ -44,6 +44,7 @@
> static svn_error_t *
> copy_versioned_files (const char *from,
> const char *to,
> + svn_opt_revision_t *revision,
> svn_boolean_t force,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> @@ -121,8 +122,8 @@
> const char *new_from = svn_path_join (from, key, subpool);
> const char *new_to = svn_path_join (to, key, subpool);
>
> - SVN_ERR (copy_versioned_files (new_from, new_to, force,
> - ctx, subpool));
> + SVN_ERR (copy_versioned_files (new_from, new_to, revision,
> + force, ctx, subpool));
> }
> }
> else if (*type == svn_node_file)
> @@ -143,8 +144,95 @@
> /* don't copy it if it isn't versioned. */
> if (entry)
> {
> - SVN_ERR (svn_io_copy_file (copy_from, copy_to, TRUE,
> - subpool));
> + svn_subst_keywords_t kw = { 0 };
> + svn_subst_eol_style_t style;
> + apr_hash_t *props;
> + const char *base;
> + svn_string_t *eol_style;
> + svn_string_t *keywords;
> + svn_string_t *executable;
> + svn_string_t *date;
> + const char *eol = NULL;
> + svn_boolean_t local_mod = FALSE;
> + apr_time_t time;
> +
> + if (revision->kind == svn_opt_revision_base)
> + {
> + SVN_ERR (svn_wc_get_pristine_copy_path
> + (copy_from, &base, subpool));
> + SVN_ERR (svn_wc_get_prop_diffs
> + (NULL, &props, copy_from, adm_access, subpool));
> + }
> + else
> + {
> + svn_wc_status_t *status;
> +
> + base = copy_from;
> + SVN_ERR (svn_wc_prop_list (&props, copy_from,
> + adm_access, subpool));
> + SVN_ERR (svn_wc_status (&status, copy_from, adm_access, subpool));
> + if (status->text_status != svn_wc_status_normal)
> + local_mod = TRUE;
> + }
> +
> + eol_style = apr_hash_get (props, SVN_PROP_EOL_STYLE,
> + APR_HASH_KEY_STRING);
> +
> + keywords = apr_hash_get (props, SVN_PROP_KEYWORDS,
> + APR_HASH_KEY_STRING);
> +
> + executable = apr_hash_get (props, SVN_PROP_EXECUTABLE,
> + APR_HASH_KEY_STRING);
> +
> + if (eol_style)
> + svn_subst_eol_style_from_value (&style, &eol,
> + eol_style->data);
> +
> + if (local_mod)
> + /* Use the modified time from the working copy if the file */
> + SVN_ERR (svn_io_file_affected_time (&time, copy_from, subpool));
> + else
> + time = entry->cmt_date;
> +
> + if (keywords)
> + {
> + const char *fmt;
> + const char *author;
> +
> + if (local_mod)
> + {
> + /* For locally modified files, we'll append an 'M'
> + to the revision number, and set the author to
> + "(local)" since we can't always determine the
> + current user's username */
> + fmt = "%" SVN_REVNUM_T_FMT "M";
> + author = "(local)";
> + }
> + else
> + {
> + fmt = "%" SVN_REVNUM_T_FMT;
> + author = entry->cmt_author;
> + }
> +
> + SVN_ERR (svn_subst_build_keywords
> + (&kw, keywords->data,
> + apr_psprintf (pool,
> + fmt,
> + entry->cmt_rev),
> + entry->url,
> + time,
> + author,
> + subpool));
> + }
> +
> + SVN_ERR (svn_subst_copy_and_translate (base, copy_to,
> + eol, FALSE,
> + &kw, TRUE,
> + subpool));
> + if (executable)
> + SVN_ERR (svn_io_set_file_executable (copy_to, TRUE, FALSE, subpool));
> +
> + SVN_ERR (svn_io_set_file_affected_time (time, copy_to, subpool));
> }
> }
>
> @@ -208,8 +296,22 @@
> svn_client_ctx_t *ctx,
> apr_pool_t *pool)
> {
> - if (svn_path_is_url (from))
> + svn_boolean_t use_ra = FALSE;
> +
> + if (! svn_path_is_url (from) &&
> + (revision->kind != svn_opt_revision_base) &&
> + (revision->kind != svn_opt_revision_working))
You could include svn_opt_revision_committed as well, because -rBASE and -rCOMMITTED necessarily refer to the same data (even though they may correspond to different revision numbers because of commits elsewhere in the repository).
But you don't have to do so.
This pattern occurs in several places in the client code; I have been looking for good ways to factor it out, but have not found much of a good way yet.
> {
> + if (revision->kind == svn_opt_revision_unspecified)
> + /* Default to WORKING in the case that we have
> + been given a working copy path */
> + revision->kind = svn_opt_revision_working;
> + else
> + use_ra = TRUE;
> + }
> +
> + if (svn_path_is_url (from) || use_ra)
> + {
> const char *URL;
> svn_revnum_t revnum;
> void *ra_baton, *session;
> @@ -219,8 +321,16 @@
> const svn_ra_reporter_t *reporter;
> void *report_baton;
>
> - URL = svn_path_canonicalize (from, pool);
> -
> + if (use_ra)
> + {
> + SVN_ERR (svn_client_url_from_path (&URL, from, pool));
> + if (! URL)
> + return svn_error_createf (SVN_ERR_ENTRY_MISSING_URL, NULL,
> + "'%s' has no URL", from);
> + }
> + else
> + URL = svn_path_canonicalize (from, pool);
> +
> SVN_ERR (svn_client__get_export_editor (&export_editor, &edit_baton,
> to, URL, force, ctx, pool));
>
> @@ -236,7 +346,7 @@
> if (revision->kind == svn_opt_revision_unspecified)
> revision->kind = svn_opt_revision_head;
> SVN_ERR (svn_client__get_revision_number
> - (&revnum, ra_lib, session, revision, to, pool));
> + (&revnum, ra_lib, session, revision, from, pool));
>
> /* Manufacture a basic 'report' to the update reporter. */
> SVN_ERR (ra_lib->do_update (session,
> @@ -273,7 +383,7 @@
> else
> {
> /* just copy the contents of the working copy into the target path. */
> - SVN_ERR (copy_versioned_files (from, to, force, ctx, pool));
> + SVN_ERR (copy_versioned_files (from, to, revision, force, ctx, pool));
> }
>
> return SVN_NO_ERROR;
> Index: subversion/clients/cmdline/main.c
> ===================================================================
> --- subversion/clients/cmdline/main.c (revision 7778)
> +++ subversion/clients/cmdline/main.c (working copy)
> @@ -253,7 +253,7 @@
> { "export", svn_cl__export, {0},
> "Create an unversioned copy of a tree.\n"
> "usage: 1. export [-r REV] URL [PATH]\n"
> - " 2. export PATH1 PATH2\n"
> + " 2. export [-r REV] PATH1 [PATH2]\n"
> "\n"
> " 1. Exports a clean directory tree from the repository specified by\n"
> " URL, at revision REV if it is given, otherwise at HEAD, into\n"
> @@ -261,8 +261,11 @@
> " for the local directory name.\n"
> "\n"
> " 2. Exports a clean directory tree from the working copy specified by\n"
> - " PATH1 into PATH2. All local changes will be preserved, but files\n"
> - " not under version control will not be copied.\n",
> + " PATH1, at revision REV if it is given, otherwise at WORKING, into\n"
> + " PATH2. If PATH2 is omitted, the last component of the PATH1 is used\n"
> + " for the local directory name. If REV is not specified, All local\n"
"All" -> "all".
> + " changes will be preserved, but files not under version control will\n"
> + " not be copied.\n",
> {'r', 'q', svn_cl__force_opt, SVN_CL__AUTH_OPTIONS,
> svn_cl__config_dir_opt} },
>
> Index: subversion/tests/clients/cmdline/export_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/export_tests.py (revision 7778)
> +++ subversion/tests/clients/cmdline/export_tests.py (working copy)
> @@ -190,6 +190,81 @@
> expected_output,
> expected_disk)
>
> +def export_working_copy_with_keyword_translation(sbox):
> + "export working copy with keyword translation"
> + sbox.build()
> +
> + wc_dir = sbox.wc_dir
> +
> + # Add a keyword to A/mu and set the svn:keywords property
> + # appropriately to make sure it's translated during
> + # the export operation
> + mu_path = os.path.join(wc_dir, 'A', 'mu')
> + svntest.main.file_append(mu_path, '$LastChangedRevision$')
> + svntest.main.run_svn(None, 'ps', 'svn:keywords',
> + 'LastChangedRevision', mu_path)
> +
> + expected_disk = svntest.main.greek_state.copy()
> + expected_disk.tweak('A/mu',
> + contents=expected_disk.desc['A/mu'].contents +
> + '$LastChangedRevision: 1M $')
> +
> + export_target = sbox.add_wc_path('export')
> +
> + svntest.actions.run_and_verify_export(wc_dir,
> + export_target,
> + svntest.wc.State(sbox.wc_dir, {}),
> + expected_disk)
> +
> +def export_working_copy_with_property_mods(sbox):
> + "export working copy with property mods"
> + sbox.build()
> +
> + wc_dir = sbox.wc_dir
> +
> + # Make a local property mod to A/my
"A/my" -> "A/mu".
> + mu_path = os.path.join(wc_dir, 'A', 'mu')
> + svntest.main.file_append(mu_path, '\n')
> + svntest.main.run_svn(None, 'ps', 'svn:eol-style',
> + 'CR', mu_path)
> +
> + expected_disk = svntest.main.greek_state.copy()
> + expected_disk.tweak('A/mu',
> + contents=expected_disk.desc['A/mu'].contents +
> + '\r')
> +
> + export_target = sbox.add_wc_path('export')
> +
> + svntest.actions.run_and_verify_export(wc_dir,
> + export_target,
> + svntest.wc.State(sbox.wc_dir, {}),
> + expected_disk)
> +
> +def export_working_copy_at_base_revision(sbox):
> + "export working copy at base revision"
> + sbox.build()
> +
> + wc_dir = sbox.wc_dir
> +
> + # Add a keyword to A/mu and set the svn:keywords property
> + # appropriately to make sure it's translated during
> + # the export operation
> + mu_path = os.path.join(wc_dir, 'A', 'mu')
> + svntest.main.file_append(mu_path, 'Appended text')
> +
> + # Note that we don't tweak the expected disk tree at all,
> + # since the appended text should not be present.
> + expected_disk = svntest.main.greek_state.copy()
> +
> + export_target = sbox.add_wc_path('export')
> +
> + svntest.actions.run_and_verify_export(wc_dir,
> + export_target,
> + svntest.wc.State(sbox.wc_dir, {}),
> + expected_disk,
> + None, None, None, None,
> + '-rBASE')
> +
> ########################################################################
> # Run the tests
>
[...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] Take 3: Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
Okay, I think I've got all of this worked. I use the working copy timestamps
only if the file is locally modified. For a locally modified file, I will
also substitute in "(local)" as the author, append an 'M' to the revision
number, and use the working copy time for LastChangedDate.
So, here's another stab with everything. If no one disapproves of it, I'll
commit it tomorrow, and hopefully move on to more serious problems. :-)
-John
Log:
----------------------------------------
Fixes a minor issue with the 'export' command failing to copy the BASE
revision of files when '-rBASE' is specified. Also fixes translation of
keywords for locally modified files, as well as adding appropriate
timestamping. Adds a couple of tests for this new functionality.
* subversion/libsvn_client/export.c
(copy_versioned_files): Added revision parameter. Changed the copy
algorithm to translate keywords and eol style when appropriate.
(svn_client_export): Added the ability to better distinguish when the
repository should be contacted versus using the local base or working copy.
* subversion/clients/cmdline/main.c
(svn_cl__cmd_table): Updated to reflect export's new functionality.
* subversion/tests/clients/cmdline/export_tests.py
(export_working_copy_with_keyword_translation,
export_working_copy_with_property_mods,
export_working_copy_at_base_revision): New export tests.
* subversion/tests/clients/cmdline/svntest/actions.py
(run_and_verify_export): Added new *args parameter to support new export
tests.
----------------------------------------
Re: [PATCH] Take 2: Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
On Tuesday 18 November 2003 16:19, Branko Čibej wrote:
> kfogel@collab.net wrote:
> >Branko Čibej <br...@xbc.nu> writes:
> >>>The system
> >>>username might be totally unrelated to the usernames used by the
> >>>repository. So $LastChangedBy$ should be unaffected, IMHO.
> >>
> >>I disagree. If export can't figure out who made the local changes, it
> >>definitely shouldn't lie about it in the keyword expansion.
> >
> >Ah, I see. That makes sense -- perhaps $LastChangedBy$ should remain
> >unexpanded, or say "<local>", or something.
I think I'm going to put "(Locally Modified)" as the author, and append the
'M' as suggest by Eric.
> >But, ... still kind of bikesheddy. I'd be happy with *any* of the
> >behaviors, including the current one. There are much more important
> >problems in Subversion :-).
>
> Hah. I'd say. Export-from-WC is one of the less blindingly critical
> features right now, for example...
Well, it didn't start out that way. :-) I wanted to beef up the test suite
and this just kind of ensued from there. But I'm hoping to get this wrapped
up soon, and if you have suggestions for something that isn't terribly
complicated, I'm all ears. I'd like to help you guys reach 1.0 as quick as
possible, and if my time can be better spent somewhere else, I'd like to know
about it.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 2: Modified export behavior
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
>Branko Čibej <br...@xbc.nu> writes:
>
>
>>>The system
>>>username might be totally unrelated to the usernames used by the
>>>repository. So $LastChangedBy$ should be unaffected, IMHO.
>>>
>>>
>>I disagree. If export can't figure out who made the local changes, it
>>definitely shouldn't lie about it in the keyword expansion.
>>
>>
>
>Ah, I see. That makes sense -- perhaps $LastChangedBy$ should remain
>unexpanded, or say "<local>", or something.
>
>But, ... still kind of bikesheddy. I'd be happy with *any* of the
>behaviors, including the current one. There are much more important
>problems in Subversion :-).
>
>
Hah. I'd say. Export-from-WC is one of the less blindingly critical
features right now, for example...
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 2: Modified export behavior
Posted by kf...@collab.net.
Branko Čibej <br...@xbc.nu> writes:
> >The system
> >username might be totally unrelated to the usernames used by the
> >repository. So $LastChangedBy$ should be unaffected, IMHO.
>
> I disagree. If export can't figure out who made the local changes, it
> definitely shouldn't lie about it in the keyword expansion.
Ah, I see. That makes sense -- perhaps $LastChangedBy$ should remain
unexpanded, or say "<local>", or something.
But, ... still kind of bikesheddy. I'd be happy with *any* of the
behaviors, including the current one. There are much more important
problems in Subversion :-).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 2: Modified export behavior
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
>The system
>username might be totally unrelated to the usernames used by the
>repository. So $LastChangedBy$ should be unaffected, IMHO.
>
>
I disagree. If export can't figure out who made the local changes, it
definitely shouldn't lie about it in the keyword expansion.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 2: Modified export behavior
Posted by kf...@collab.net.
John Szakmeister <jo...@szakmeister.net> writes:
> The next problem I ran into was time-stamping the file. In an export
> from the repository, you'd get the last committed time stamps. So, I
> did the same for export... except for one problem, what do I do with
> locally modified files? At the moment, I'm making a call to
> svn_wc_status() and checking text_status or props_status != normal.
> If either has been modified, then I'm pulling the time stamp from the
> WC file and applying it to the export.
>
> Then I started thinking, that if I copied the modified file times from
> the working copy, that someone might use that information to determine
> what had changed. Then I got to wondering whether or not the
> $LastChangedRev$ should reflect the fact that a file is locally
> modified (perhaps something like $LastChangedRev: 2+ $), as well as
> changing $LastChangedBy$ to reflect the current user.
Whew.
First, the timestamps: I'd say if the file's text has been modified
locally, then the timestamp should be that of the working file. If
only props -- even a keyword or eol prop -- are modified, then use the
committed time, because exports do not preserve properties (even
though they may preserve keyword or eol effects, they don't preserve
the actual properties).
As for the keyword expansions: you can't count on the "current" user
having any meaning in the repository username space. The system
username might be totally unrelated to the usernames used by the
repository. So $LastChangedBy$ should be unaffected, IMHO.
As for the rev, if you want to put a "+", I think that's okay, but
it's sort of a bikeshed to me... :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 2: Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
On Tuesday 18 November 2003 05:52, John Szakmeister wrote:
> Oof, found a problem with this... the svn:executable property doesn't get
> applied. I've got to track down how to do that now.
Okay, I fixed this problem, but it brought around more questions, that I feel
I need a little guidance on.
The next problem I ran into was time-stamping the file. In an export from the
repository, you'd get the last committed time stamps. So, I did the same for
export... except for one problem, what do I do with locally modified files?
At the moment, I'm making a call to svn_wc_status() and checking text_status
or props_status != normal. If either has been modified, then I'm pulling the
time stamp from the WC file and applying it to the export.
Then I started thinking, that if I copied the modified file times from the
working copy, that someone might use that information to determine what had
changed. Then I got to wondering whether or not the $LastChangedRev$ should
reflect the fact that a file is locally modified (perhaps something like
$LastChangedRev: 2+ $), as well as changing $LastChangedBy$ to reflect the
current user.
So now, I'm a bit confused as to what I should do. :-) I want this to do the
intuitive thing, but I'm not sure what that is. Any help that can be offered
would be appreciated.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Take 2: Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
Oof, found a problem with this... the svn:executable property doesn't get
applied. I've got to track down how to do that now.
-John
On Monday 17 November 2003 19:03, John Szakmeister wrote:
> Here's the new patch complete with tests and all. Although, I'm a native
> english speaker, I'm not terribly elegant with command summaries. :-)
> Please have a look and let me know if it's okay. :-)
>
> -John
>
> Log:
> ---------------------------------
> Fixes a minor issue with the 'export' command failing to copy the BASE
> revision of files when '-rBASE' is specified. Also fixes translation of
> keywords for locally modified files. Adds a couple of tests for this new
> functionality.
>
> * subversion/libsvn_client/export.c
> (copy_versioned_files): Added revision parameter. Changed the copy
> algorithm to translate keywords and eol style when appropriate.
>
> (svn_client_export): Added the ability to better distinguish when the
> repository should be contacted versus using the local base or working
> copy.
>
> * subversion/clients/cmdline/main.c
> (svn_cl__cmd_table): Updated to reflect export's new functionality.
>
> * subversion/tests/clients/cmdline/export_tests.py
> (export_working_copy_with_keyword_translation,
> export_working_copy_with_property_mods,
> export_working_copy_at_base_revision): New export tests.
>
> * subversion/tests/clients/cmdline/svntest/actions.py
> (run_and_verify_export): Added new *args parameter to support new export
> tests.
>
> ---------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] Take 2: Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
Here's the new patch complete with tests and all. Although, I'm a native
english speaker, I'm not terribly elegant with command summaries. :-) Please
have a look and let me know if it's okay. :-)
-John
Log:
---------------------------------
Fixes a minor issue with the 'export' command failing to copy the BASE
revision of files when '-rBASE' is specified. Also fixes translation of
keywords for locally modified files. Adds a couple of tests for this new
functionality.
* subversion/libsvn_client/export.c
(copy_versioned_files): Added revision parameter. Changed the copy
algorithm to translate keywords and eol style when appropriate.
(svn_client_export): Added the ability to better distinguish when the
repository should be contacted versus using the local base or working copy.
* subversion/clients/cmdline/main.c
(svn_cl__cmd_table): Updated to reflect export's new functionality.
* subversion/tests/clients/cmdline/export_tests.py
(export_working_copy_with_keyword_translation,
export_working_copy_with_property_mods,
export_working_copy_at_base_revision): New export tests.
* subversion/tests/clients/cmdline/svntest/actions.py
(run_and_verify_export): Added new *args parameter to support new export
tests.
---------------------------------
Re: [PATCH] Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
On Monday 17 November 2003 13:05, Julian Foad wrote:
[snip]
> You mean, when you export from WC (not from BASE), the keywords that refer
> to "this revision" will take their values from the BASE revision, because
> there is no "this revision" yet? That sounds like a reasonable behaviour,
> but I just want to ask whether that situation arises with any other
> commands and, if so, do they handle it the same way?
Yep, that's it. The only other command that I think would operate this way is
'cat', which I see you mentioned below.
> (The only one I can think of is "svn cat -rWC" which is not yet
> implemented, for which there is a patch attached to issue 1361: svn cat
> -rBASE contacts repository. I can't remember what it does in this
> situation, but it can be changed to match your choice.)
Sounds good.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
On Sunday 16 November 2003 13:22, Philip Martin wrote:
> Philip Martin <ph...@codematters.co.uk> writes:
> > John Szakmeister <jo...@szakmeister.net> writes:
> >> On Sunday 16 November 2003 12:57, Philip Martin wrote:
> >>> Did you copy this pattern from elsewhere in the code?
> >>
> >> Actually I did. I saw this pattern being used in svn_client_cat() in
> >> libsvn_client/cat.c.
> >
> > far better to pass some sort of "binaryness" indicator to
> > copy_and_translate.
>
> Or perhaps just call svn_client_cat?
Right now, svn_client_cat() goes to the repository for everything. This gets
rid of the use case that I'm trying to fix. :-) But it is an interesting
idea.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
On Sunday 16 November 2003 16:40, kfogel@collab.net wrote:
> Philip Martin <ph...@codematters.co.uk> writes:
> > We have client side code that prevents those keywords being set on
> > binary files. As far as I know, it doesn't prevent someone setting
> > the keywords on a non-binary file, and then changing the file to
> > binary. It's not clear to me whether we should ignore the keywords on
> > a binary file. If we do choose to ignore them I am a little uneasy
> > about the behaviour being distributed in lots of places in the code,
> > far better to pass some sort of "binaryness" indicator to
> > copy_and_translate.
>
> I don't like the idea of ignoring them at all. Surely there are
> binary formats in the world that can tolerate (say) keyword expansion.
Good point.
> This does not contradict the client-side user protections. Having
> client-side code to prevent setting translation properties on binary
> files makes sense, because it's far more likely to happen accidentally
> than not. One can still override with -F, or by a workaround of
> temporarily resetting the mime-type, if they really need to.
>
> Which means that if we *do* receive a translation property on a
> non-text file, someone probably went through extra trouble to put it
> there! Therefore, we shouldn't ignore it.
I'll remove that stuff from the patch then. I'll also fix svn cat while I'm
at it (but I'll put that in a separate patch/commit).
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:
> Philip Martin <ph...@codematters.co.uk> writes:
>> We have client side code that prevents those keywords being set on
>> binary files. As far as I know, it doesn't prevent someone setting
>> the keywords on a non-binary file, and then changing the file to
>> binary. It's not clear to me whether we should ignore the keywords on
>> a binary file. If we do choose to ignore them I am a little uneasy
>> about the behaviour being distributed in lots of places in the code,
>> far better to pass some sort of "binaryness" indicator to
>> copy_and_translate.
>
> I don't like the idea of ignoring them at all. Surely there are
> binary formats in the world that can tolerate (say) keyword expansion.
I suspect such formats do exist.
> This does not contradict the client-side user protections. Having
> client-side code to prevent setting translation properties on binary
> files makes sense, because it's far more likely to happen accidentally
> than not. One can still override with -F, or by a workaround of
> temporarily resetting the mime-type, if they really need to.
>
> Which means that if we *do* receive a translation property on a
> non-text file, someone probably went through extra trouble to put it
> there! Therefore, we shouldn't ignore it.
As far as I can see update will apply translations to binary files if
the appropriate properties are set. That means it's probably a bug
that svn cat behaves differently.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by kf...@collab.net.
Philip Martin <ph...@codematters.co.uk> writes:
> We have client side code that prevents those keywords being set on
> binary files. As far as I know, it doesn't prevent someone setting
> the keywords on a non-binary file, and then changing the file to
> binary. It's not clear to me whether we should ignore the keywords on
> a binary file. If we do choose to ignore them I am a little uneasy
> about the behaviour being distributed in lots of places in the code,
> far better to pass some sort of "binaryness" indicator to
> copy_and_translate.
I don't like the idea of ignoring them at all. Surely there are
binary formats in the world that can tolerate (say) keyword expansion.
This does not contradict the client-side user protections. Having
client-side code to prevent setting translation properties on binary
files makes sense, because it's far more likely to happen accidentally
than not. One can still override with -F, or by a workaround of
temporarily resetting the mime-type, if they really need to.
Which means that if we *do* receive a translation property on a
non-text file, someone probably went through extra trouble to put it
there! Therefore, we shouldn't ignore it.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:
> John Szakmeister <jo...@szakmeister.net> writes:
>
>> On Sunday 16 November 2003 12:57, Philip Martin wrote:
>>>
>>> Did you copy this pattern from elsewhere in the code?
>>
>> Actually I did. I saw this pattern being used in svn_client_cat() in
>> libsvn_client/cat.c.
>
> far better to pass some sort of "binaryness" indicator to
> copy_and_translate.
Or perhaps just call svn_client_cat?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by Philip Martin <ph...@codematters.co.uk>.
John Szakmeister <jo...@szakmeister.net> writes:
> On Sunday 16 November 2003 12:57, Philip Martin wrote:
>>
>> I don't think the "if binary do this else do that" is necessary.
>> Binary files won't have keywords, eol-style and copy_and_translate
>> will just do a copy_file. Did you copy this pattern from elsewhere in
>> the code?
>
> Actually I did. I saw this pattern being used in svn_client_cat() in
> libsvn_client/cat.c. I had thought about removing it, but my impression was
> that it was some sort of safety precaution against someone specifying
> keywords on a binary file. If that's not the case, I'll chuck it.
We have client side code that prevents those keywords being set on
binary files. As far as I know, it doesn't prevent someone setting
the keywords on a non-binary file, and then changing the file to
binary. It's not clear to me whether we should ignore the keywords on
a binary file. If we do choose to ignore them I am a little uneasy
about the behaviour being distributed in lots of places in the code,
far better to pass some sort of "binaryness" indicator to
copy_and_translate.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
On Sunday 16 November 2003 12:57, Philip Martin wrote:
> John Szakmeister <jo...@szakmeister.net> writes:
> > + if (mime_type && svn_mime_type_is_binary
> > (mime_type->data)) + {
> > + SVN_ERR (svn_io_copy_file (base,
> > + copy_to,
> > + TRUE,
> > + subpool));
> > + }
>
> I don't think the "if binary do this else do that" is necessary.
> Binary files won't have keywords, eol-style and copy_and_translate
> will just do a copy_file. Did you copy this pattern from elsewhere in
> the code?
Actually I did. I saw this pattern being used in svn_client_cat() in
libsvn_client/cat.c. I had thought about removing it, but my impression was
that it was some sort of safety precaution against someone specifying
keywords on a binary file. If that's not the case, I'll chuck it.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by Philip Martin <ph...@codematters.co.uk>.
John Szakmeister <jo...@szakmeister.net> writes:
> + if (mime_type && svn_mime_type_is_binary (mime_type->data))
> + {
> + SVN_ERR (svn_io_copy_file (base,
> + copy_to,
> + TRUE,
> + subpool));
> + }
I don't think the "if binary do this else do that" is necessary.
Binary files won't have keywords, eol-style and copy_and_translate
will just do a copy_file. Did you copy this pattern from elsewhere in
the code?
> + else
> + {
> + svn_string_t *eol_style;
> + svn_string_t *keywords;
> + const char *eol = NULL;
> +
> + eol_style = apr_hash_get (props, SVN_PROP_EOL_STYLE,
> + APR_HASH_KEY_STRING);
> +
> + keywords = apr_hash_get (props, SVN_PROP_KEYWORDS,
> + APR_HASH_KEY_STRING);
> +
> + if (eol_style)
> + svn_subst_eol_style_from_value (&style, &eol,
> + eol_style->data);
> +
> + if (keywords)
> + {
> + SVN_ERR (svn_subst_build_keywords
> + (&kw, keywords->data,
> + apr_psprintf (pool,
> + "%" SVN_REVNUM_T_FMT,
> + entry->cmt_rev),
> + entry->url,
> + entry->cmt_date,
> + entry->cmt_author,
> + subpool));
> + }
> +
> + SVN_ERR (svn_subst_copy_and_translate (base, copy_to,
> + eol, FALSE,
> + &kw, TRUE,
> + subpool));
> + }
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Modified export behavior
Posted by Julian Foad <ju...@btopenworld.com>.
John Szakmeister wrote:
> Okay, I've went and modified the export behavior to do a couple of things now:
>
> 1) Now translates keywords when doing an export of a working copy.
>
> 2) Make better use of -r REV and a working copy path. You can now specify
> -rBASE on the command line and export will translate and copy the text base
> instead of the working copy files which may have been modified.
>
> 3) In the case that -r REV can't be satisfied by the working copy path (i.e.,
> PREV), then we'll contact the repository to satisfy the request.
That all sounds good.
> For the keyword translation of the locally modified working copy, I used the
> new properties on the file, but things like $LastChangedBy$ and
> $LastChangedRev$ fall back to using the values associated with the commit.
You mean, when you export from WC (not from BASE), the keywords that refer to "this revision" will take their values from the BASE revision, because there is no "this revision" yet? That sounds like a reasonable behaviour, but I just want to ask whether that situation arises with any other commands and, if so, do they handle it the same way?
(The only one I can think of is "svn cat -rWC" which is not yet implemented, for which there is a patch attached to issue 1361: svn cat -rBASE contacts repository. I can't remember what it does in this situation, but it can be changed to match your choice.)
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] Modified export behavior
Posted by John Szakmeister <jo...@szakmeister.net>.
Okay, I've went and modified the export behavior to do a couple of things now:
1) Now translates keywords when doing an export of a working copy.
2) Make better use of -r REV and a working copy path. You can now specify
-rBASE on the command line and export will translate and copy the text base
instead of the working copy files which may have been modified.
3) In the case that -r REV can't be satisfied by the working copy path (i.e.,
PREV), then we'll contact the repository to satisfy the request.
For the keyword translation of the locally modified working copy, I used the
new properties on the file, but things like $LastChangedBy$ and
$LastChangedRev$ fall back to using the values associated with the commit.
If there are no objections, then I'll commit this in a day or two. BTW, I've
also got some new export tests for this new functionality. I'll look at the
help strings for export too. If anyone has any suggestions, I'd appreciate
it!
-John
Log:
-------------------------------------------
Fixes a minor issue with the 'export' command failing to copy the BASE
revision of files when '-rBASE' is specified. Also fixes translation of
keywords for locally modified files.
* subversion/libsvn_client/export.c
(copy_versioned_files): Added revision parameter. Changed the copy
algorithm to translate keywords and eol style when appropriate.
(svn_client_export): Added the ability to better distinguish when the
repository should be contacted versus using the local base or working copy.
-------------------------------------------
Re: Export behavior (was Re: [PATCH] svn export needs tests (issue 506))
Posted by John Szakmeister <jo...@szakmeister.net>.
On Sunday 09 November 2003 21:39, John Szakmeister wrote:
> On Sunday 09 November 2003 14:49, Garrett Rooney wrote:
> > On Nov 9, 2003, at 2:46 PM, John Szakmeister wrote:
> > > The strange part is that it uses the working text and the base props.
> > > If it
> > > did was entirely one way or the other (either base text/props or
> > > working
> > > text/props), I'd say it wasn't an issue. But the fact that it's
> > > mixed, makes
> > > me want to treat this like a bug.
> >
> > Yeah, it's a bug.
>
> I agree. I'll take a look at it next week after I get back from the DDK
> conference. If it isn't too complicated, I'll take care of it. If it
> turns out to be more work than I can handle, I'll file an issue for it. I
> don't think that's going to be the case though.
Hmph. I was just perusing the code and it seems that as long as the 'from'
path is a working copy path, it *always* exports WORKING, even if -rX is
specified on the command line. On the plus side, it doesn't look like it's
going to be too bad to fix.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Export behavior (was Re: [PATCH] svn export needs tests (issue 506))
Posted by John Szakmeister <jo...@szakmeister.net>.
On Sunday 09 November 2003 14:49, Garrett Rooney wrote:
> On Nov 9, 2003, at 2:46 PM, John Szakmeister wrote:
> > The strange part is that it uses the working text and the base props.
> > If it
> > did was entirely one way or the other (either base text/props or
> > working
> > text/props), I'd say it wasn't an issue. But the fact that it's
> > mixed, makes
> > me want to treat this like a bug.
>
> Yeah, it's a bug.
I agree. I'll take a look at it next week after I get back from the DDK
conference. If it isn't too complicated, I'll take care of it. If it turns
out to be more work than I can handle, I'll file an issue for it. I don't
think that's going to be the case though.
>
> > I'm not sure I understand the working copy thing either, or than you
> > can avoid
> > having to touch the server again to generate a clean base from which to
> > package your product.
>
> The one real use case I've heard is for testing something where the end
> result of your build can't have .svn directories in it, like a package
> building system (I think the debian package build was the example I'd
> heard). If there are .svn directories in there, some tool complains
> about it, so you export from your working copy to a clean tree and test
> it there. That way you can test your changes before committing them.
Yeah, I can see how that'd be useful.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Export behavior (was Re: [PATCH] svn export needs tests (issue 506))
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Nov 9, 2003, at 2:46 PM, John Szakmeister wrote:
> The strange part is that it uses the working text and the base props.
> If it
> did was entirely one way or the other (either base text/props or
> working
> text/props), I'd say it wasn't an issue. But the fact that it's
> mixed, makes
> me want to treat this like a bug.
Yeah, it's a bug.
> I'm not sure I understand the working copy thing either, or than you
> can avoid
> having to touch the server again to generate a clean base from which to
> package your product.
The one real use case I've heard is for testing something where the end
result of your build can't have .svn directories in it, like a package
building system (I think the debian package build was the example I'd
heard). If there are .svn directories in there, some tool complains
about it, so you export from your working copy to a clean tree and test
it there. That way you can test your changes before committing them.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Export behavior (was Re: [PATCH] svn export needs tests (issue 506))
Posted by John Szakmeister <jo...@szakmeister.net>.
On Sunday 09 November 2003 13:23, kfogel@collab.net wrote:
> John Szakmeister <jo...@szakmeister.net> writes:
> > That was actually a great idea, it seems to have brought up a couple of
> > issues. Let's say I modify my working copy so that A/mu now has a
> > property set on it (svn:eol-style) and I've appended new text to it. If
> > I export the working copy, should I see the new property I set get
> > applied? I ask, because right now if I export a working copy that has
> > modified properties (but not committed), I don't see the new properties
> > applied in the export operation. The documentation for export says "All
> > local changes will be preserved, but files not under revision control
> > will not be copied." Well, technically setting a property but not
> > committing it is a local change, and I'm not seeing it get exported.
> >
> > So what's the right behavior here? Should export be applying these new
> > properties? If not, then should we update the documentation to reflect
> > this fact? If export should be applying these properties, should I open
> > an issue?
>
> Yeah, I think it should use the working props & text, not the base
> props & text. But I have to admit, I've never really understood the
> use cases for our "export a working copy" functionality, and so
> wouldn't consider these to be pre-1.0 bugs (just IMHO).
The strange part is that it uses the working text and the base props. If it
did was entirely one way or the other (either base text/props or working
text/props), I'd say it wasn't an issue. But the fact that it's mixed, makes
me want to treat this like a bug.
I'm not sure I understand the working copy thing either, or than you can avoid
having to touch the server again to generate a clean base from which to
package your product.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Export behavior (was Re: [PATCH] svn export needs tests (issue 506))
Posted by kf...@collab.net.
John Szakmeister <jo...@szakmeister.net> writes:
> That was actually a great idea, it seems to have brought up a couple of
> issues. Let's say I modify my working copy so that A/mu now has a property
> set on it (svn:eol-style) and I've appended new text to it. If I export the
> working copy, should I see the new property I set get applied? I ask,
> because right now if I export a working copy that has modified properties
> (but not committed), I don't see the new properties applied in the export
> operation. The documentation for export says "All local changes will be
> preserved, but files not under revision control will not be copied." Well,
> technically setting a property but not committing it is a local change, and
> I'm not seeing it get exported.
>
> So what's the right behavior here? Should export be applying these new
> properties? If not, then should we update the documentation to reflect this
> fact? If export should be applying these properties, should I open an issue?
Yeah, I think it should use the working props & text, not the base
props & text. But I have to admit, I've never really understood the
use cases for our "export a working copy" functionality, and so
wouldn't consider these to be pre-1.0 bugs (just IMHO).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Export behavior (was Re: [PATCH] svn export needs tests (issue 506))
Posted by John Szakmeister <jo...@szakmeister.net>.
On Saturday 08 November 2003 21:42, kfogel@collab.net wrote:
[snip]
> Those sound good; I can't think of anything else off the top of my
> head... Oh: maybe, exports that include eol translation and/or keyword
> substitution?
That was actually a great idea, it seems to have brought up a couple of
issues. Let's say I modify my working copy so that A/mu now has a property
set on it (svn:eol-style) and I've appended new text to it. If I export the
working copy, should I see the new property I set get applied? I ask,
because right now if I export a working copy that has modified properties
(but not committed), I don't see the new properties applied in the export
operation. The documentation for export says "All local changes will be
preserved, but files not under revision control will not be copied." Well,
technically setting a property but not committing it is a local change, and
I'm not seeing it get exported.
So what's the right behavior here? Should export be applying these new
properties? If not, then should we update the documentation to reflect this
fact? If export should be applying these properties, should I open an issue?
As a side note, I've added 5 new tests in r7680. Once we work out the correct
behavior of export with local property changes, I'll put in several more
tests to ensure that it does what is expected in those cases.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn export needs tests (issue 506)
Posted by kf...@collab.net.
John Szakmeister <jo...@szakmeister.net> writes:
> Okay, I removed the deletion of the working copy, and added a couple
> of other comments in the run_and_verify_export() function.
>
> Commited the updated version in r7673! I'll update the issue
> tracker to show that at least there was some progress made towards
> issue #506. :-)
Perfect. Thanks, John!
> I think I'm going to work on is exporting from a working copy with local
> changes, and one to try and export into a directory that already
> exists. Any other ideas for export tests?
Those sound good; I can't think of anything else off the top of my
head... Oh: maybe, exports that include eol translation and/or keyword
substitution?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn export needs tests (issue 506)
Posted by John Szakmeister <jo...@szakmeister.net>.
Okay, I removed the deletion of the working copy, and added a couple of other
comments in the run_and_verify_export() function.
Commited the updated version in r7673! I'll update the issue tracker to show
that at least there was some progress made towards issue #506. :-)
I think I'm going to work on is exporting from a working copy with local
changes, and one to try and export into a directory that already exists. Any
other ideas for export tests?
-John
On Friday 07 November 2003 10:37, kfogel@collab.net wrote:
> John Szakmeister <jo...@szakmeister.net> writes:
> > I'll pull that portion out and re-submit the patch later tonight.
> >
> > Thanks for the review!
>
> Sure -- as long as you're redoing it, just go ahead and commit it
> (i.e., I won't apply the old patch this morning, then).
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn export needs tests (issue 506)
Posted by kf...@collab.net.
John Szakmeister <jo...@szakmeister.net> writes:
> I'll pull that portion out and re-submit the patch later tonight.
>
> Thanks for the review!
Sure -- as long as you're redoing it, just go ahead and commit it
(i.e., I won't apply the old patch this morning, then).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn export needs tests (issue 506)
Posted by John Szakmeister <jo...@szakmeister.net>.
On Thursday 06 November 2003 23:21, kfogel@collab.net wrote:
> John Szakmeister <jo...@szakmeister.net> writes:
[snip]
> One minor question, in the new actions.py:run_and_verify_export():
> > + if isinstance(output_tree, wc.State):
> > + output_tree = output_tree.old_tree()
> > + if isinstance(disk_tree, wc.State):
> > + disk_tree = disk_tree.old_tree()
> > +
> > + # Remove dir if it's already there.
> > + main.safe_rmtree(wc_dir_name)
>
> You already remove the target in the caller. Are you sure you want to
> remove it here? We might someday have a test that tests what happens
> when you export over an existing directory, for example.
>
> I realize that you're probably copying from run_and_verify_checkout(),
> which does the same thing, and I have no idea why it does. Especially
> since it's doc string doesn't mention the behavior :-).
>
> I read mail on a different machine than my working copy, but will
> apply this in the morning.
Uh, well, I cheated and borrowed from the run_and_verify_export(). :-) I
actually wondered about that a bit myself, but figured there must have been
some reason for it... so I left the call there. I don't see any harm in
removing it.
I'll pull that portion out and re-submit the patch later tonight.
Thanks for the review!
-John
> Thanks!
>
> -Karl
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] svn export needs tests (issue 506)
Posted by kf...@collab.net.
John Szakmeister <jo...@szakmeister.net> writes:
> Log:
> ---------------------------------
> A step towards finishing issue #506. Adds a test of an exported greek tree.
>
> * subversion/tests/clients/cmdline/export_tests.py
> (export_greek_tree): New.
> (test_list): Added new export_greek_tree test to the list.
>
> * subversion/tests/clients/cmdline/svntest/actions.py
> (run_and_verify_export): New.
This looks good to me!
One minor question, in the new actions.py:run_and_verify_export():
> + if isinstance(output_tree, wc.State):
> + output_tree = output_tree.old_tree()
> + if isinstance(disk_tree, wc.State):
> + disk_tree = disk_tree.old_tree()
> +
> + # Remove dir if it's already there.
> + main.safe_rmtree(wc_dir_name)
You already remove the target in the caller. Are you sure you want to
remove it here? We might someday have a test that tests what happens
when you export over an existing directory, for example.
I realize that you're probably copying from run_and_verify_checkout(),
which does the same thing, and I have no idea why it does. Especially
since it's doc string doesn't mention the behavior :-).
I read mail on a different machine than my working copy, but will
apply this in the morning.
Thanks!
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org