You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pa...@softlanding.com> on 2006/06/15 18:01:30 UTC
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Hi All,
After a delay working on some other things I had a chance to rework this
patch based on your various suggestions. While there are a lot of little
tweaks (e.g. improved comments, new Python tests) the following are the
major changes from the previous patch:
----------------------------------------
1) "Takeover" is no more, the term that is. This patch is about
tolerating obstructions to added items, so to avoid confusion I've removed
all reference to "takeover" in the patch. This doesn't actually have much
of a user-visible impact, just that the notification of obstructions is no
longer 'T'akeover but rather 'E' for 'already Exists' (Thanks Philip).
----------------------------------------
2) Obstruction tolerance is now available using --force for update and
switch as well as checkout. The --help messages for sw, up, and co
describe the new behavior.
----------------------------------------
3) Obstructing versioned directory obstructions are no longer allowed and
always cause an error.
----------------------------------------
4) svn_client_cleanup() is no longer used to "fix" the entries file so
that 'E'xisting WC files with no actual modifications have their text-time
field set the file's last modified time. Instead I revved
svn_wc_text_modified_p() so it has the option to use the base revision in
tmp when checking for mods. Now we can not only check if an obstructing
file differs from a file about to be added, but svn_wc_text_modified_p()
cleans up the entries file:
From svn_wc__text_modified_internal_p():
/* It is quite legitimate for modifications to the working copy to
produce a timestamp variation with no text variation. If it turns out
that there are no differences then we might be able to "repair" the
text-time in the entries file and so avoid the expensive file
contents
comparison in the future. */
if (! *modified_p && svn_wc_adm_locked(adm_access))
{
svn_wc_entry_t tmp;
SVN_ERR(svn_io_file_affected_time(&tmp.text_time, filename, pool));
SVN_ERR(svn_wc__entry_modify(adm_access,
svn_path_basename(filename, pool),
&tmp, SVN_WC__ENTRY_MODIFY_TEXT_TIME,
TRUE,
pool));
}
Sidebar: I noticed that this "repair" behavior is not described in the doc
string for svn_wc_text_modified_p(), should it be?
----------------------------------------
5) Eliminated references to the command line client in the svn_wc_* APIs
and code.
----------------------------------------
Please take a look if you have some time. Any and all feedback
appreciated, thanks,
Paul B.
[[[
Support --force option with svn checkout, update, and switch.
With the force option, co, sw, and up now tolerate unversioned
obstructing paths when adding new paths of the same type, rather than
generating a SVN_ERR_WC_OBSTRUCTED_UPDATE error.
If the obstructing path is the same type (file or directory) as the
corresponding path in the repository it will be left 'as-is' in the
working copy. For directories this simply means the obstruction is
tolerated. For files, any content differences between the obstruction
and the repository are treated like a local modification to the
working copy.
This patch is an expansion of one originally posted by Jonathan Gilbert
<o2...@sneakemail.com> against the 1.2.0 tag. See the various
"Takeover" threads on the dev mailing list.
* build.conf
(test-scripts): Add checkout_tests.py.
* subversion/include/svn_client.h
(svn_client_checkout3, svn_client_update3,
svn_client_switch2): New.
(svn_client_checkout2, svn_client_update2,
svn_client_switch): Deprecate.
* subversion/include/svn_wc.h
(svn_wc_notify_action_t): New 'Exists' action.
(svn_wc_get_update_editor3, svn_wc_text_modified_p2,
svn_wc_get_switch_editor3): New.
(svn_wc_get_update_editor2, svn_wc_text_modified_p,
svn_wc_get_switch_editor2): Deprecate.
* subversion/libsvn_client/checkout.c
(svn_client__checkout_internal): New argument. Update calls to
svn_client__update_internal.
(svn_client_checkout3): New.
(svn_client_checkout): Reimplement as wrapper around
svn_client_checkout3.
(svn_client_checkout): Update call to svn_client__checkout_internal.
* subversion/libsvn_client/client.h
(svn_client__update_internal, svn_client__checkout_internal,
svn_client__switch_internal): New bool argument to identify updates,
checkouts, and switches, respectively, made with --force option.
* subversion/libsvn_client/copy.c
(repos_to_wc_copy): Update call to svn_client__checkout_internal.
* subversion/libsvn_client/externals.c
(switch_external): Update calls to svn_client__checkout_internal,
svn_client__update_internal, and svn_client__switch_internal.
(handle_external_item_change): Update call to
svn_client__checkout_internal.
* subversion/libsvn_client/switch.c
(svn_client__switch_internal): New argument. Replace call to
svn_wc_get_switch_editor2 with svn_wc_get_switch_editor3.
(svn_client_switch2): New.
(svn_client_switch): Update call to svn_client__switch_internal.
* subversion/libsvn_client/update.c
(svn_client__update_internal): New argument. Replace call to
svn_wc_get_update_editor2 with svn_wc_get_update_editor3.
(svn_client_update3): New.
(svn_client_update2): Reimplement as wrapper around
svn_client_update3.
(svn_client_update): Update call to svn_client__update_internal.
* subversion/libsvn_wc/adm_ops.c
(revert_admin_things): Update call to
svn_wc__text_modified_internal_p.
* subversion/libsvn_client/commit_util.c (harvest_committables):
* subversion/libsvn_wc/diff.c (file_diff, close_file):
* subversion/libsvn_wc/log.c (svn_wc_cleanup2):
* subversion/libsvn_wc/status.c (assemble_status):
Replace calls to svn_wc_text_modified_p with svn_wc_text_modified_p2.
* subversion/libsvn_wc/props.c
(svn_wc_prop_set2): Update comment.
* subversion/libsvn_wc/questions.c
(svn_wc__text_modified_internal_p): New arg. Update call to
svn_wc__text_base_path.
(svn_wc_text_modified_p2): New. Update comment.
(svn_wc_text_modified_p): Reimplement as wrapper around
svn_wc_text_modified_p2
* subversion/libsvn_wc/update_editor.c
(struct edit_baton): New member allow_obstructions.
(struct file_baton): New member existed.
(make_file_baton): Initialize new file_baton member.
(add_directory): Allow obstructing directories if edit baton
permits them. Support notification for adding directories which
already 'E'xist.
(add_or_open_file): Allow obstructing files if edit baton
permits them.
(change_file_prop): Cache the last-changed-date propval for
obstructing files.
(merge_file): New argument to identify when obstructions are
allowed. Handle the various obstruction scenarios.
(close_file): Update call to merge_file. Support notification for
adding files which already 'E'xist.
(make_editor): New argument to identify when obstructions are
allowed. Initialize new edit baton member.
(svn_wc_get_update_editor3) New.
(svn_wc_get_update_editor, svn_wc_get_update_editor2): Reimplement
as wrappers around svn_wc_get_update_editor3.
(svn_wc_get_switch_editor3): New.
(svn_wc_get_switch_editor, svn_wc_get_switch_editor2): Reimplement
as wrappers around svn_wc_get_switch_editor3.
* subversion/libsvn_wc/wc.h
(svn_wc__text_modified_internal_p): New arg.
* subversion/svn/checkout-cmd.c
(svn_cl__checkout): Replace call to svn_wc_get_update_editor2 with
svn_wc_get_update_editor3.
* subversion/svn/main.c
(svn_cl__cmd_table): Enable --force option with svn co, up, and sw.
Add new help text for each.
* subversion/svn/notify.c
(notify): Handle new 'Existed' action.
* subversion/svn/switch-cmd.c
(svn_cl__switch): Replace call to svn_client_switch with
svn_client_switch2.
* subversion/svn/update-cmd.c
(svn_cl__update): Replace call to svn_client_update2 with
svn_client_update3.
* subversion/tests/cmdline/checkout_tests.py: New.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Update to reflect new svn switch help text.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_checkout): Support optional arguments. Don't delete
WC target during forced checkouts.
* subversion/tests/cmdline/svntest/main.py
(run_and_verify_switch): Support optional arguments and regular
expression error checking.
* subversion/tests/cmdline/svntest/tree.py
(build_tree_from_checkout): Enable parsing of 'E' notification.
* subversion/tests/cmdline/switch_tests.py
(forced_switch, forced_switch_failures): New test definitions.
(test_list): Run new tests.
* subversion/tests/cmdline/update_tests.py
(forced_update, forced_update_failures): New test definitions.
(test_list): Run new tests.
]]]
_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 6/23/06, Paul Burba <pa...@softlanding.com> wrote:
> Ok, assuming I'm understanding you correctly(?) I looked at all the
> instances of "raise svntest.Failure" and found we actually do do one of
> the following some frequency:
>
> A) print 'some explanation of the error'
> raise svntest.Failure
>
> B) raise svntest.Failure('some explanation of the error')
>
> Regardless of what's been done, where is the harm in providing a bit more
> information to someone reading the log? Perhaps this doesn't matter in
> most circumstances since the tests usually pass and I doubt people spend
> much time pouring over the logs. But in my own experience porting
> Subversion to OS/400 I've read more logs than I care to remember; and
> tests that raised failures with no explanation made finding the underlying
> problem more difficult than necessary.
>
> Anyhow, I'm not married to the additional message, but I'd prefer it if
> there are no strong objections.
>
> FWIW I changed my patch to use approach B rather than A. I didn't realize
> svntest.Failure supported the message.
I hadn't realized we did things both ways. Personally, I prefer
version B, but that's just me. Thanks for fixing the other problems
though.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Paul Burba <pa...@softlanding.com>.
rooneg@gmail.com wrote on 06/22/2006 10:49:26 AM:
> On 6/22/06, Paul Burba <pa...@softlanding.com> wrote:
> > Hi All,
> >
> > I'm reposting this patch in the hope someone might be able to take a
look.
> > It is a fairly complex change compared to what I've done before (at
least
> > outside of the OS400/EBCDIC port) so I'd like to have another set of
eyes
> > look at it.
Hi Garrett,
Thanks for taking a look.
> I just went through the diff, and it looks fine to me... I also built
> it and tried it out, and some of the new checkout tests are failing
> (number 2 and 7). I believe it's because I'm building in maintainer
> mode and the line numbers for errors are screwing up the regexes
> looking for the right error message.
Fixed that.
> Also, those tests print random stuff out to stdout when we have
> problems, we don't usually do that.
By "have problems" do you mean printing a message and raising
svntest.Failure?
e.g.:
# Checkout the standard greek repos into a directory that has a dir
named
# "iota" obstructing the file "iota" in the repos. This should fail.
sout, serr = svntest.actions.run_and_verify_svn("Expected error during
co",
None, SVNAnyOutput,
"co",
"--force",
sbox.repo_url,
other_wc)
expected_error = "svn: Failed to add file.*a non-file object of the " +
\
"same name already exists"
if not re.compile(expected_error).search(serr[0]):
print "forced co failed but not in the expected way"
raise svntest.Failure
Ok, assuming I'm understanding you correctly(?) I looked at all the
instances of "raise svntest.Failure" and found we actually do do one of
the following some frequency:
A) print 'some explanation of the error'
raise svntest.Failure
B) raise svntest.Failure('some explanation of the error')
Regardless of what's been done, where is the harm in providing a bit more
information to someone reading the log? Perhaps this doesn't matter in
most circumstances since the tests usually pass and I doubt people spend
much time pouring over the logs. But in my own experience porting
Subversion to OS/400 I've read more logs than I care to remember; and
tests that raised failures with no explanation made finding the underlying
problem more difficult than necessary.
Anyhow, I'm not married to the additional message, but I'd prefer it if
there are no strong objections.
FWIW I changed my patch to use approach B rather than A. I didn't realize
svntest.Failure supported the message.
Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 6/22/06, Paul Burba <pa...@softlanding.com> wrote:
> Hi All,
>
> I'm reposting this patch in the hope someone might be able to take a look.
> It is a fairly complex change compared to what I've done before (at least
> outside of the OS400/EBCDIC port) so I'd like to have another set of eyes
> look at it.
I just went through the diff, and it looks fine to me... I also built
it and tried it out, and some of the new checkout tests are failing
(number 2 and 7). I believe it's because I'm building in maintainer
mode and the line numbers for errors are screwing up the regexes
looking for the right error message.
Also, those tests print random stuff out to stdout when we have
problems, we don't usually do that.
Good work in general though, it'll be good to get this done.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 08/01/2006 05:59:27 PM:
> Paul Burba wrote:
> > Discussion of this patch fell by the wayside when I went on vacation.
I
> > believe this is ready to commit, but I wanted to check if anyone has
any
> > outstanding concerns regarding it. I'm in no major rush, but I'd like
to
> > get it into trunk.
>
> Hi Paul. I think it's time you checked in this patch. It's had
> ample time for
> review even though I still haven't got around to going through it as
> thouroughly as I would like to in an ideal world. But it's not
likeyou're a
> drive-by contributor: you're around to improve it later if people find
things
> that need your input. So just go for it. Enough people agree the
> functionality is good, and any deficiencies that remain by now must be
minor.
Yes, I'm too cautious at times, better than too reckless :-) Committed
r20945.
> Some trivia I noticed:
>
> * main.c: Help text for "update" ends with a colon.
>
> * checkout_tests.py: whole content repeated in the patch file: take
care
> when applying the patch in case the unversioned file already exists,and
check
> you've "svn add"ed the file before committing.
>
> * checkout_tests.py: "'-m', 'Log message for new import'" is
> silly (I'd just
> write 'Import' or '').
>
> * checkout_tests.py: "Julian's scenario" -> proper brief description
> (something like "Ensure that an import followed by a checkout in place
works
> perfectly")
>
> * Some spaces at end of lines (now we're talking Julian-level trivia)
Fixed all.
> > Julian - I know you wanted to see a new option used rather than
--force,
> > but looking back in the takeover threads I don't see a firm consensus
on
> > this. If a new option is eventually what we decide on, I'll galdly
change
> > it later.
>
> I'm fine with putting it in as "--force" for now. I'd rather like it if
it
> could be renamed before it gets released, but that's for a separate
> discussion.
Ok, leaving this for a future debate.
Thanks for all your help,
Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> Discussion of this patch fell by the wayside when I went on vacation. I
> believe this is ready to commit, but I wanted to check if anyone has any
> outstanding concerns regarding it. I'm in no major rush, but I'd like to
> get it into trunk.
Hi Paul. I think it's time you checked in this patch. It's had ample time for
review even though I still haven't got around to going through it as
thouroughly as I would like to in an ideal world. But it's not like you're a
drive-by contributor: you're around to improve it later if people find things
that need your input. So just go for it. Enough people agree the
functionality is good, and any deficiencies that remain by now must be minor.
Some trivia I noticed:
* main.c: Help text for "update" ends with a colon.
* checkout_tests.py: whole content repeated in the patch file: take care
when applying the patch in case the unversioned file already exists, and check
you've "svn add"ed the file before committing.
* checkout_tests.py: "'-m', 'Log message for new import'" is silly (I'd just
write 'Import' or '').
* checkout_tests.py: "Julian's scenario" -> proper brief description
(something like "Ensure that an import followed by a checkout in place works
perfectly")
* Some spaces at end of lines (now we're talking Julian-level trivia)
> Julian - I know you wanted to see a new option used rather than --force,
> but looking back in the takeover threads I don't see a firm consensus on
> this. If a new option is eventually what we decide on, I'll galdly change
> it later.
I'm fine with putting it in as "--force" for now. I'd rather like it if it
could be renamed before it gets released, but that's for a separate discussion.
Thanks,
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Paul Burba <pa...@softlanding.com>.
Hi All,
Discussion of this patch fell by the wayside when I went on vacation. I
believe this is ready to commit, but I wanted to check if anyone has any
outstanding concerns regarding it. I'm in no major rush, but I'd like to
get it into trunk.
Julian - I know you wanted to see a new option used rather than --force,
but looking back in the takeover threads I don't see a firm consensus on
this. If a new option is eventually what we decide on, I'll galdly change
it later.
Thanks,
Paul B.
[[[
Support --force option with svn checkout, update, and switch.
With the force option, co, sw, and up now tolerate unversioned
obstructing paths when adding new paths of the same type, rather than
generating a SVN_ERR_WC_OBSTRUCTED_UPDATE error.
If the obstructing path is the same type (file or directory) as the
corresponding path in the repository it will be left 'as-is' in the
working copy. For directories this simply means the obstruction is
tolerated. For files, any content differences between the obstruction
and the repository are treated like a local modification to the
working copy.
This patch is an expansion of one originally posted by Jonathan Gilbert
<o2...@sneakemail.com> against the 1.2.0 tag. See the various
"Takeover" threads on the dev mailing list.
* build.conf
(test-scripts): Add checkout_tests.py.
* subversion/include/svn_client.h
(svn_client_checkout3, svn_client_update3,
svn_client_switch2): New.
(svn_client_checkout2, svn_client_update2,
svn_client_switch): Deprecate.
* subversion/include/svn_wc.h
(svn_wc_notify_action_t): New 'Exists' action.
(svn_wc_get_update_editor3, svn_wc_text_modified_p2,
svn_wc_get_switch_editor3): New.
(svn_wc_get_update_editor2, svn_wc_text_modified_p,
svn_wc_get_switch_editor2): Deprecate.
* subversion/libsvn_client/checkout.c
(svn_client__checkout_internal): New argument. Update calls to
svn_client__update_internal.
(svn_client_checkout3): New.
(svn_client_checkout2, svn_client_checkout): Update calls to
svn_client__checkout_internal.
* subversion/libsvn_client/client.h
(svn_client__update_internal, svn_client__checkout_internal,
svn_client__switch_internal): New bool argument to identify updates,
checkouts, and switches, respectively, made with --force option.
* subversion/libsvn_client/copy.c
(repos_to_wc_copy): Update call to svn_client__checkout_internal.
* subversion/libsvn_client/externals.c
(switch_external): Update calls to svn_client__checkout_internal,
svn_client__update_internal, and svn_client__switch_internal.
(handle_external_item_change): Update call to
svn_client__checkout_internal.
* subversion/libsvn_client/switch.c
(svn_client__switch_internal): New argument. Replace call to
svn_wc_get_switch_editor2 with svn_wc_get_switch_editor3.
(svn_client_switch2): New.
(svn_client_switch): Update call to svn_client__switch_internal.
* subversion/libsvn_client/update.c
(svn_client__update_internal): New argument. Replace call to
svn_wc_get_update_editor2 with svn_wc_get_update_editor3.
(svn_client_update3): New.
(svn_client_update2): Reimplement as wrapper around
svn_client_update3.
(svn_client_update): Update call to svn_client__update_internal.
* subversion/libsvn_wc/adm_ops.c
(revert_admin_things): Update call to
svn_wc__text_modified_internal_p.
* subversion/libsvn_client/commit_util.c (harvest_committables):
* subversion/libsvn_wc/diff.c (file_diff, close_file):
* subversion/libsvn_wc/log.c (svn_wc_cleanup2):
* subversion/libsvn_wc/status.c (assemble_status):
Replace calls to svn_wc_text_modified_p with svn_wc_text_modified_p2.
* subversion/libsvn_wc/props.c
(svn_wc_prop_set2): Update comment.
* subversion/libsvn_wc/questions.c
(svn_wc__text_modified_internal_p): New arg. Update call to
svn_wc__text_base_path.
(svn_wc_text_modified_p2): New. Update comment.
(svn_wc_text_modified_p): Reimplement as wrapper around
svn_wc_text_modified_p2
* subversion/libsvn_wc/update_editor.c
(struct edit_baton): New member allow_unver_obstructions.
(struct file_baton): New member existed.
(make_file_baton): Initialize new file_baton member.
(add_directory): Allow obstructing directories if edit baton
permits them. Support notification for adding directories which
already 'E'xist.
(add_or_open_file): Allow obstructing files if edit baton
permits them.
(change_file_prop): Cache the last-changed-date propval for
obstructing files.
(merge_file): New argument to identify when obstructions are
allowed. Handle the various obstruction scenarios.
(close_file): Update call to merge_file. Support notification for
adding files which already 'E'xist.
(make_editor): New argument to identify when obstructions are
allowed. Initialize new edit baton member.
(svn_wc_get_update_editor3) New.
(svn_wc_get_update_editor, svn_wc_get_update_editor2): Reimplement
as wrappers around svn_wc_get_update_editor3.
(svn_wc_get_switch_editor3): New.
(svn_wc_get_switch_editor, svn_wc_get_switch_editor2): Reimplement
as wrappers around svn_wc_get_switch_editor3.
* subversion/libsvn_wc/wc.h
(svn_wc__text_modified_internal_p): New arg to select comparison
against normal text base or temporary text base.
* subversion/svn/checkout-cmd.c
(svn_cl__checkout): Replace call to svn_wc_get_update_editor2 with
svn_wc_get_update_editor3.
* subversion/svn/main.c
(svn_cl__cmd_table): Enable --force option with svn co, up, and sw.
Add new help text for each.
* subversion/svn/notify.c
(notify): Handle new 'Existed' action.
* subversion/svn/switch-cmd.c
(svn_cl__switch): Replace call to svn_client_switch with
svn_client_switch2.
* subversion/svn/update-cmd.c
(svn_cl__update): Replace call to svn_client_update2 with
svn_client_update3.
* subversion/tests/cmdline/checkout_tests.py: New.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Update to reflect new svn switch help text.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_checkout): Support optional arguments. Don't delete
WC target during forced checkouts.
* subversion/tests/cmdline/svntest/main.py
(run_and_verify_switch): Support optional arguments and regular
expression error checking.
* subversion/tests/cmdline/svntest/tree.py
(build_tree_from_checkout): Enable parsing of 'E' notification.
* subversion/tests/cmdline/switch_tests.py
(forced_switch, forced_switch_failures): New test definitions.
(test_list): Run new tests.
* subversion/tests/cmdline/update_tests.py
(forced_update, forced_update_failures): New test definitions.
(test_list): Run new tests.
]]]
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 06/29/2006 08:38:50 PM:
> Paul Burba wrote:
> > Julian Foad <ju...@btopenworld.com> wrote on 06/27/2006 07:51:21
PM:
> [...]
> >>We should seriously consider adding a more specific command-line
> option name
> >>too. It's pure chance that the three commands you wanted to add
> >>this behaviour
> >>to didn't already have "--force" options for some other purpose.
> >
> > One man's pure chance is another's serendipity. Seriously, I have no
> > strong objections to a new option, but wouldn't this be a problem with
> > *any* new behavior associated with --force? Is it simply the
ill-defined
> > nature of --force in general that concerns you?
> > http://svn.haxx.se/dev/archive-2006-05/0143.shtml
>
> Yes, I advocate creating a new, well-defined command-line option foreach
new
> behaviour, not just this one.
So really, you'd prefer that --force never be added to any subcommand
which doesn't already support it right? On a scale of 1 [don't care] to
10 [vehemently opposed, will do all in your power to thwart :-)] how
strongly do you feel about this patch using --force? I have no great
argument in favor of --force other than it seems appropriate for what I'm
doing here and is available. Long story short, if you really feel
strongly about it I'll add a new option. --allow-obstructions maybe?
> Have a good trip to the woods and don't do computer stuff unless
> you're really
> stuck for anything else!
It didn't rain much, so I didn't use my laptop much, except to confirm
that it wouldn't rain :-)
> Teach yourself a new song or a poem, or try to
> whittle yourself a wooden spoon using a knife...
No luck with the spoon, song, or poem, but I did learn 2 new things.
First, cycling up Crawford and Pinkham notches makes one think fondly of
internal combustion. Second, when deep in the woods, even small bears who
immediately run *away* from you, are still quite scary when they burst
unexpectedly from the underbrush.
Julian Foad <ju...@btopenworld.com> wrote on 06/27/2006 07:51:21 PM:
> Right, that's all for tonight; I'll try to do some more but it probably
won't
> be tomorrow. Feel free to send an update in the meantime. Thanks
> for listening...
Here is a new patch, mostly it's just catching up with the latest activity
on trunk (r20569). There is a small tweak to the checkout_test.py in test
#8 import_and_checkout that fixes a failure when running the tests via
ra_svn.
Let me know if you had any other suggestions or any outstanding comments
on my previous update http://svn.haxx.se/dev/archive-2006-06/0951.shtml.
Thanks,
Paul B.
[[[
Support --force option with svn checkout, update, and switch.
With the force option, co, sw, and up now tolerate unversioned
obstructing paths when adding new paths of the same type, rather than
generating a SVN_ERR_WC_OBSTRUCTED_UPDATE error.
If the obstructing path is the same type (file or directory) as the
corresponding path in the repository it will be left 'as-is' in the
working copy. For directories this simply means the obstruction is
tolerated. For files, any content differences between the obstruction
and the repository are treated like a local modification to the
working copy.
This patch is an expansion of one originally posted by Jonathan Gilbert
<o2...@sneakemail.com> against the 1.2.0 tag. See the various
"Takeover" threads on the dev mailing list.
* build.conf
(test-scripts): Add checkout_tests.py.
* subversion/include/svn_client.h
(svn_client_checkout3, svn_client_update3,
svn_client_switch2): New.
(svn_client_checkout2, svn_client_update2,
svn_client_switch): Deprecate.
* subversion/include/svn_wc.h
(svn_wc_notify_action_t): New 'Exists' action.
(svn_wc_get_update_editor3, svn_wc_text_modified_p2,
svn_wc_get_switch_editor3): New.
(svn_wc_get_update_editor2, svn_wc_text_modified_p,
svn_wc_get_switch_editor2): Deprecate.
* subversion/libsvn_client/checkout.c
(svn_client__checkout_internal): New argument. Update calls to
svn_client__update_internal.
(svn_client_checkout3): New.
(svn_client_checkout2, svn_client_checkout): Update calls to
svn_client__checkout_internal.
* subversion/libsvn_client/client.h
(svn_client__update_internal, svn_client__checkout_internal,
svn_client__switch_internal): New bool argument to identify updates,
checkouts, and switches, respectively, made with --force option.
* subversion/libsvn_client/copy.c
(repos_to_wc_copy): Update call to svn_client__checkout_internal.
* subversion/libsvn_client/externals.c
(switch_external): Update calls to svn_client__checkout_internal,
svn_client__update_internal, and svn_client__switch_internal.
(handle_external_item_change): Update call to
svn_client__checkout_internal.
* subversion/libsvn_client/switch.c
(svn_client__switch_internal): New argument. Replace call to
svn_wc_get_switch_editor2 with svn_wc_get_switch_editor3.
(svn_client_switch2): New.
(svn_client_switch): Update call to svn_client__switch_internal.
* subversion/libsvn_client/update.c
(svn_client__update_internal): New argument. Replace call to
svn_wc_get_update_editor2 with svn_wc_get_update_editor3.
(svn_client_update3): New.
(svn_client_update2): Reimplement as wrapper around
svn_client_update3.
(svn_client_update): Update call to svn_client__update_internal.
* subversion/libsvn_wc/adm_ops.c
(revert_admin_things): Update call to
svn_wc__text_modified_internal_p.
* subversion/libsvn_client/commit_util.c (harvest_committables):
* subversion/libsvn_wc/diff.c (file_diff, close_file):
* subversion/libsvn_wc/log.c (svn_wc_cleanup2):
* subversion/libsvn_wc/status.c (assemble_status):
Replace calls to svn_wc_text_modified_p with svn_wc_text_modified_p2.
* subversion/libsvn_wc/props.c
(svn_wc_prop_set2): Update comment.
* subversion/libsvn_wc/questions.c
(svn_wc__text_modified_internal_p): New arg. Update call to
svn_wc__text_base_path.
(svn_wc_text_modified_p2): New. Update comment.
(svn_wc_text_modified_p): Reimplement as wrapper around
svn_wc_text_modified_p2
* subversion/libsvn_wc/update_editor.c
(struct edit_baton): New member allow_unver_obstructions.
(struct file_baton): New member existed.
(make_file_baton): Initialize new file_baton member.
(add_directory): Allow obstructing directories if edit baton
permits them. Support notification for adding directories which
already 'E'xist.
(add_or_open_file): Allow obstructing files if edit baton
permits them.
(change_file_prop): Cache the last-changed-date propval for
obstructing files.
(merge_file): New argument to identify when obstructions are
allowed. Handle the various obstruction scenarios.
(close_file): Update call to merge_file. Support notification for
adding files which already 'E'xist.
(make_editor): New argument to identify when obstructions are
allowed. Initialize new edit baton member.
(svn_wc_get_update_editor3) New.
(svn_wc_get_update_editor, svn_wc_get_update_editor2): Reimplement
as wrappers around svn_wc_get_update_editor3.
(svn_wc_get_switch_editor3): New.
(svn_wc_get_switch_editor, svn_wc_get_switch_editor2): Reimplement
as wrappers around svn_wc_get_switch_editor3.
* subversion/libsvn_wc/wc.h
(svn_wc__text_modified_internal_p): New arg to select comparison
against normal text base or temporary text base.
* subversion/svn/checkout-cmd.c
(svn_cl__checkout): Replace call to svn_wc_get_update_editor2 with
svn_wc_get_update_editor3.
* subversion/svn/main.c
(svn_cl__cmd_table): Enable --force option with svn co, up, and sw.
Add new help text for each.
* subversion/svn/notify.c
(notify): Handle new 'Existed' action.
* subversion/svn/switch-cmd.c
(svn_cl__switch): Replace call to svn_client_switch with
svn_client_switch2.
* subversion/svn/update-cmd.c
(svn_cl__update): Replace call to svn_client_update2 with
svn_client_update3.
* subversion/tests/cmdline/checkout_tests.py: New.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Update to reflect new svn switch help text.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_checkout): Support optional arguments. Don't delete
WC target during forced checkouts.
* subversion/tests/cmdline/svntest/main.py
(run_and_verify_switch): Support optional arguments and regular
expression error checking.
* subversion/tests/cmdline/svntest/tree.py
(build_tree_from_checkout): Enable parsing of 'E' notification.
* subversion/tests/cmdline/switch_tests.py
(forced_switch, forced_switch_failures): New test definitions.
(test_list): Run new tests.
* subversion/tests/cmdline/update_tests.py
(forced_update, forced_update_failures): New test definitions.
(test_list): Run new tests.
]]]
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> Julian Foad <ju...@btopenworld.com> wrote on 06/27/2006 07:51:21 PM:
[...]
>>We should seriously consider adding a more specific command-line option name
>>too. It's pure chance that the three commands you wanted to add
>>this behaviour
>>to didn't already have "--force" options for some other purpose.
>
> One man's pure chance is another's serendipity. Seriously, I have no
> strong objections to a new option, but wouldn't this be a problem with
> *any* new behavior associated with --force? Is it simply the ill-defined
> nature of --force in general that concerns you?
> http://svn.haxx.se/dev/archive-2006-05/0143.shtml
Yes, I advocate creating a new, well-defined command-line option for each new
behaviour, not just this one.
Have a good trip to the woods and don't do computer stuff unless you're really
stuck for anything else! Teach yourself a new song or a poem, or try to
whittle yourself a wooden spoon using a knife...
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 06/27/2006 07:51:21 PM:
> Paul Burba wrote:
> > Julian Foad <ju...@btopenworld.com> wrote on 06/22/2006 07:12:16
PM:
> >>Paul Burba wrote:
> >
> >>>* subversion/libsvn_wc/questions.c
> >>> (svn_wc__text_modified_internal_p): New arg. Update call to
> >>> svn_wc__text_base_path.
> >>> (svn_wc_text_modified_p2): New. Update comment.
> >>> (svn_wc_text_modified_p): Reimplement as wrapper around
> >>> svn_wc_text_modified_p2
> >>
> >>I'm not keen on adding a boolean parameter to any API. Calls
withmultiple
> >>booleans quickly become unreadable. But it's not a big deal here
> >>compared with some of our other APIs.
> >>
> >>Thinking out loud... I do wonder whether providing operations on
> thetemporary
> >>text base is something we really want to be exposing in this way.
> >
> > Are you thinking a new function to handle comparisons with the temp
text
> > base is better or that operating on the temp text base is a
fundamentally
> > flawed design? If the latter, do you have any specific concerns?
>
> Actually I wasn't thinking that deeply at all - it was just a "first
> impression" that providing APIs to access a "temporary" object is
> either a bad
> idea our should be done thoroughly. I do see in this patch that the
vast
> majority of calls to this function don't want to access the temporary
copy,
> which would seem to suggest that keeping an API without that option
> is sensible.
>
> I'll hopefully think more about this later, and reply in another mail.
Ok, I leave it as is for now.
> > In a nutshell, I'm using svn_wc_text_modified_p2() this way because:
While
> > not absolutely necessary, it's beneficial to "fix" the entries file so
> > obstructing files with no actual differences have their text-time set
to
> > the file's last modified time. Presently the patch relies on
> > svn_wc_text_modified_p() to do this, but can only do so if it can
operate
> > on files about to be added (i.e. whose text base exists only in the
temp
> > area).
> >
> >>Are we providing the option to access the "tmp" text base in all other
> >>libsvn_wc APIs that access the text base? (I haven't looked.)
> >
> > No, this is something new.
>
> >>>checkout (co): Check out a working copy from a repository.
> >>>usage: checkout URL[@REV]... [PATH]
> >>
> >>[...]
> > I see your point, how does this sound?
> >
> > If --force is used, unversioned obstructing paths in the working
> > copy destination do not automatically cause the check out to fail.
> > If the obstructing path is the same type (file or directory) as the
> > corresponding path in the repository it becomes versioned but its
> > contents are left 'as-is' in the working copy. This means that an
> > obstructing directory's unversioned children may also obstruct and
> > become versioned. For files, any content differences between the
> > obstruction and the repository are treated like a local modification
> > to the working copy.
>
> That text seems good.
Added similar text to switch and update as well.
> [...]
> > Sorry bout that, the bit about "applicable auto-props" is a mistake on
my
> > part. The patch as it stands now does *not* add any auto-props, it
just
> > applies the props from the repository, just as sw, up, and co normally
do
> > when adding files.
>
> OK.
>
> >>I think it would make
> >>sense to give the working file the properties that "svn add" would
> have given
> >>it, and then any differences between that set and the pristine
> base set from
> >>the repository will show up as local modifications. To me that
> would be the
> >>equivalent for properties of what we do with the base text, and
> therefore the
> >>right thing to do.
> >
> > Regardless of the equivalency between treating props this way and how
text
> > is treated, I don't see it as desirable behavior. Do we really want
to
> > lose all the props from the repository? Is there a common use case
where
> > this is preferable?
>
> What I was suggesting is that the BASE props come from the repository
and the
> WORKING props come from the WC's automatic rules (including "auto-props"
and
> auto-detection of executable, mime-type, etc.), just as if the user
> had "added"
> or "imported" the file. I suggest that this mirrors the fact that the
BASE
> text comes from the repos and the WORKING text from the WC. Thus no
> properties
> are "lost"; it is up to the user to observe any differences and
> decide what to
> do about them, just like with the text. In some situations I feel this
would
> be the logical thing to do, but I'm unable to think of a good example.
Ah, that is what you said the first time...I misread it.
> The alternative, and I think this is what you are suggesting, is to set
the
> working props equal to the base props from the repository. I believe
this
> makes sense if the expected use case is when the obstructing files and
> directories have been obtained from the repository and perhaps modified
> somewhat, but not been replaced with different types of files
(executability,
> binaryness, etc.).
>
> Now that I think about it, I agree that the latter behaviour is much
more
> likely to be useful in most cases, and I'm happy with it.
Ok, we'll go with the existing approach.
> >>> For each item checked out a line will start with a character
> >>> reporting the action taken. For unforced checkouts this character
> >>> is always 'A' for 'Added'. Forced checkouts report obstructed
paths
> >>> with 'E' for 'Existed' and use 'A' otherwise.
> >>
> >>This paragraph is talking about files Added by the checkout, but
> doesn't say
> >>so, incorrectly implying that no other codes than 'E' and 'A' can
> >>result from a checkout.
> >
> > Talk about missing the forest for the trees, I got so caught up with
> > initial checkouts into unversioned dirs that I forgot about the
> > update-esque behavior of doing subsequent checkouts into an existing
WC.
>
> Heh, I can quite see how one could get into that situation with a
> patch of this
> size ... but isn't it me getting into a muddle this time? Isn't it
correct
> that a checkout can only add files? It's updates (and switches) that
can do
> other things.
>
> > Perhaps the best approach is to simply not mention the notification
codes
> > as you suggested with switch below? I've done just that for now,
unless
> > anyone feels strongly that 'E' warrants special mention in co's help.
>
> If I'm right that I was wrong, put it back! Sorry for the confusion. If
I'm
> wrong now and was right before, please enlighten me. :-)
Heh, I think your wrong now and was right before! My initial reaction
was, "svn co can only 'A'dd paths! Julian must be crazy!" :-) But
subsequent checkouts are essentially updates, so 'D', 'U', 'C', and 'G'
can all make an appearence:
Initial checkout:
svn co file:///home/BURBA/REPOS/CO_MANIA /home/BURBA/WCS/CO_MAINIA_WC
A \home\BURBA\WCS\CO_MAINIA_WC\Doc A.txt
A \home\BURBA\WCS\CO_MAINIA_WC\Doc B.txt
A \home\BURBA\WCS\CO_MAINIA_WC\Doc C.txt
A \home\BURBA\WCS\CO_MAINIA_WC\Doc D.txt
A \home\BURBA\WCS\CO_MAINIA_WC\Doc E.txt
Checked out revision 1.
Make some changes to repos via another WC:
svn ci -m "" "/home/BURBA/WCS/CO_MAINIA_WC1"
Deleting home\BURBA\WCS\CO_MAINIA_WC1\Doc A.txt
Sending home\BURBA\WCS\CO_MAINIA_WC1\Doc B.txt
Sending home\BURBA\WCS\CO_MAINIA_WC1\Doc C.txt
Sending home\BURBA\WCS\CO_MAINIA_WC1\Doc E.txt
Transmitting file data ...
Committed revision 2.
Checkout into first WC again:
svn co file:///home/BURBA/REPOS/CO_MANIA /home/BURBA/WCS/CO_MAINIA_WC
D \home\BURBA\WCS\CO_MAINIA_WC\Doc A.txt
C \home\BURBA\WCS\CO_MAINIA_WC\Doc B.txt
U \home\BURBA\WCS\CO_MAINIA_WC\Doc C.txt
G \home\BURBA\WCS\CO_MAINIA_WC\Doc E.txt
Checked out revision 2.
> I think we've got the concept sorted now, and all that remains is a few
> implementation details :-)
>
> There follows a code review of about half of the patch. I've run out of
time
> tonight and thought it would be best to post what I have written so far.
>
>
> > Index: subversion/include/svn_client.h
> > ===================================================================
> > --- subversion/include/svn_client.h (revision 20268)
> > +++ subversion/include/svn_client.h (working copy)
> > @@ -696,15 +696,40 @@
> > * just the directory represented by @a URL and its immediate
> > * non-directory children, but none of its child directories (if
any).
> > *
> > + * If @a force_checkout is @c TRUE then checkout tolerates an
> existing local
> > + * tree rooted at @a PATH even if some paths in the local tree
> are identical
>
> This comment should probably say "unversioned" somewhere, like the other
APIs
> and the command-line help do.
Done.
> "some paths ... are identical": The word "path" is sometimes used tomean
the
> item existing at a given path, so this could be misleading. I suggest
making
> the wording consistent with your doc strings for "update" and "switch"
which
> don't use the word "identical" and are clearer.
Agreed, that doc string used my original language from when the patch
handled only co. It also used the term "colliding" rather than
"obstructing", which is inconsistent with the rest of the patch. I
improved the doc string re the force argument and made it consistent
across svn_client_checkout3(), svn_client_update3(), and
svn_client_switch2().
> It would be really nice for the APIs to use a more descriptive parameter
name
> than "force" (and the "_checkout" suffix doesn't help). Think: it's
quite
> likely that we'll add a way to force some other aspect of its
> behaviour later.
> Maybe use "{allow,tolerate,integrate,take_over}_unver_{items,
> obstructions}" ?
You taunt me I think :-) Anything but the 'T' word! It exists only in
this thread's name...and I'd like to keep it that way, it really describes
something more elaborate than what I'm doing here.
> I don't think most of those combinations are too long given that
callers
> don't need to type it. Ah, I see in svn_wc_get_*_editor you've called
it
> "allow_obstructions". That's much better than "force", but only
> some kinds of
> obstructions are allowed. How about using "allow_unver_obstructions"?
> Whichever you choose, try to use the same name in all places except
> when there
> is a reason not to, especially in all the public APIs that are affected.
Easy enough, "allow_unver_obstructions" it is. I changed this everywhere,
I saw no reason not to.
> We should seriously consider adding a more specific command-line option
name
> too. It's pure chance that the three commands you wanted to add
> this behaviour
> to didn't already have "--force" options for some other purpose.
One man's pure chance is another's serendipity. Seriously, I have no
strong objections to a new option, but wouldn't this be a problem with
*any* new behavior associated with --force? Is it simply the ill-defined
nature of --force in general that concerns you?
http://svn.haxx.se/dev/archive-2006-05/0143.shtml
> Syle nit: Personally I hate reading "is @c FALSE" and prefer "is false",
and
> the same with "true" and "null", as these three are well-known concepts
that
> don't require references to the particular preprocessor macros that we
use to
> represent them. In Doxygen output, of course, the "@c" is
> suppressed and small
> capitals are used so I hardly notice it, but normally I'm reading
> the C header
> text. However, both styles are used extensively at present so it's your
> choice. (Apologies: I just can't stop myself from ranting on these
sorts of
> trivia!)
I never read the Doxygen output either so I'm quite happy to use the more
readable format you suggest. I was defaulting to what I thought was the
strict Doxygen standard and hadn't realized that "true", "false", etc.
were ok.
> > + * to those in @a URL. File paths that collide will effectively
treat the
> > + * local file as a user modification to the pristine checkout. If @a
force
> > + * is @c FALSE then the checkout will abort if there are any
> colliding paths.
>
> Maybe also say that working properties are set equal to the base
properties?
> That doesn't seem implicit to me.
Fixed there and added to the doc string for svn_client_update3() and
svn_client_switch2() too.
> > + *
> > * If @a URL refers to a file rather than a directory, return the
> > * error @c SVN_ERR_UNSUPPORTED_FEATURE. If @a URL does not exist,
> > * return the error @c SVN_ERR_RA_ILLEGAL_URL.
> > *
> > * Use @a pool for any temporary allocation.
> > *
> > - * @since New in 1.2.
> > + * @since New in 1.5.
> > */
> > svn_error_t *
> > +svn_client_checkout3(svn_revnum_t *result_rev,
> > + const char *URL,
> > + const char *path,
> > + const svn_opt_revision_t *peg_revision,
> > + const svn_opt_revision_t *revision,
> > + svn_boolean_t recurse,
> > + svn_boolean_t ignore_externals,
> > + svn_boolean_t force_checkout,
> > + svn_client_ctx_t *ctx,
> > + apr_pool_t *pool);
> > +
> > +
> > +/**
> > + * Similar to svn_client_checkout3() but with the force parameter
always
>
> Give the exact name of the parameter.
Fixed.
> > + * set to @c FALSE.
> > + *
> > + * @deprecated Provided for backward compatibility with the 1.4 API.
> > + */
> > +svn_error_t *
> > svn_client_checkout2(svn_revnum_t *result_rev,
> > const char *URL,
> > const char *path,
> [...]
>
> > Index: subversion/libsvn_client/externals.c
> > ===================================================================
>
> So externals won't yet support this mode. I guess that's reasonable...
I've
> never actually used them so have not been much interested in their
semantics.
I'm open to supporting it but haven't looked into any of the implications
yet (if there are any).
> > Index: subversion/libsvn_wc/update_editor.c
> > ===================================================================
> [...]
> > @@ -1057,15 +1066,65 @@
> > || ((! copyfrom_path) &&
(SVN_IS_VALID_REVNUM(copyfrom_revision))))
> > abort();
> >
> > - /* There should be nothing with this name. */
> > SVN_ERR(svn_io_check_path(db->path, &kind, db->pool));
> > - if (kind != svn_node_none)
> > - return svn_error_createf
> > - (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> > - _("Failed to add directory '%s': "
> > - "object of the same name already exists"),
> > - svn_path_local_style(db->path, pool));
> >
> > + /* Check for obstructions. */
> > + if (eb->allow_obstructions)
> > + {
> > + /* The path can exist, but it must be a directory... */
> > + if (kind == svn_node_file || kind == svn_node_unknown)
>
> "(kind != svn_node_dir)" would answer the question more directly.
At this point we only know that obstructions are allowed, there might
*not* be an obstruction though, so svn_node_none or svn_node_dir are both
acceptable.
> > + return svn_error_createf
> > + (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> > + _("Failed to add directory '%s': a non-directory object of
the "
> > + "same name already exists"),
> > + svn_path_local_style(db->path, pool));
> > + else if (kind == svn_node_dir)
>
> ... and this "else if" is redundant either way.
Fixed.
> > + {
> > + /* ...Ok, it's a directory but it can't be versioned. We
don't
> > + handle that, yet... */
> > + svn_wc_adm_access_t *adm_access;
> > + const char *obstructing_path = svn_path_join(
> > + pb->path, svn_path_basename(path, pool), pool);
>
> Why is obstructing_path not just db->path? That's the path that we
found of
> kind svn_node_dir.
Yes that works, my reasoning for recreating is lost in the haze of time.
> > +
> > + /* Test the obstructing dir to see if it's versioned. */
> > + svn_error_t *err = svn_wc_adm_open3(&adm_access, NULL,
> > + obstructing_path,
FALSE, 0,
> > + NULL, NULL, pool);
> > + if (err && err->apr_err != SVN_ERR_WC_NOT_DIRECTORY)
> > + {
> > + /* Something quite unexepected has happened. */
> > + return err;
> > + }
> > + else if (err)
> > + {
> > + /* Obstructing dir is not versioned, just need to flag
it as
> > + existing then we are done here. */
> > + exists = TRUE;
> > + svn_error_clear(err);
> > + }
> > + else
> > + {
> > + /* Obstructing dir *is* versioned. */
> > + return svn_error_createf
> > + (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> > + _("Failed to forcibly add directory '%s': a
versioned "
> > + "directory of the same name already exists"),
>
> Minor nit: I don't think inclusion of the word "forcibly" helps,
> especially if
> the operation was invoked through a client interface other than "svn
> co --force".
Removed "forcibly".
> > + svn_path_local_style(db->path, pool));
> > + }
> [...]
> > @@ -1684,9 +1764,9 @@
> > applied. */
> > fb->prop_changed = 1;
> >
> > - /* Special case: if the file is added during a checkout, cache the
> > - last-changed-date propval for future use. */
> > - if (eb->use_commit_times
> > + /* Special case: if the file is added during a checkout or existed,
cache
> > + the last-changed-date propval for future use. */
> > + if ((eb->use_commit_times || fb->existed)
>
> Are you sure about this change? It looks odd to me, looks like the code
> doesn't match the comment, even before your change. Does "if
> (use_commit_times)" really mean "if (file is added during a checkout)"?
Ignoring my patch for a moment, that comment is not very enlightening.
use_commit_times really means the miscellaneous config variable
"use-commit-times" is "yes" - see r6983. The comment should probably be
something more like this:
- /* Special case: if the file is added during a checkout, cache the
- last-changed-date propval for future use. */
+ /* Special case: If use-commit-times config variable is set we
+ cache the last-changed-date propval so we can use it to set
+ the working file's timestamp. */
As for the patch, the reason I cache the last changed date is so that it's
available later in merge_file(). There it's used to set the text-time for
(differing) obstructing files. Otherwise the obstruction would have no
text-time entry. I suppose this isn't necessary(?) I need to think on
that more.
/* It is quite legitimate for modifications to the working copy to
produce a timestamp variation with no text variation. If it turns out
that there are no differences then we might be able to "repair" the
text-time in the entries file and so avoid the expensive file
contents
comparison in the future. */
> > && (strcmp(name, SVN_PROP_ENTRY_COMMITTED_DATE) == 0)
> > && value)
> > fb->last_changed_date = apr_pstrdup(fb->pool, value->data);
> [...]
>
> Right, that's all for tonight; I'll try to do some more but it probably
won't
> be tomorrow. Feel free to send an update in the meantime. Thanks
> for listening...
...Thanks for the review. Here is an updated log and patch reflecting
your latest suggestions.
FYI: I'm on vacation in the deep woods of New Hampshire from 6/30 - 7/10
and should have e-mail access via dial-up during that time. I'll be
checking my messages, but probably not every day. Barring any technical
difficulties, my Subversion activity level during that time will largely
be a function of how much it rains :-P
Paul B.
[[[
Support --force option with svn checkout, update, and switch.
With the force option, co, sw, and up now tolerate unversioned
obstructing paths when adding new paths of the same type, rather than
generating a SVN_ERR_WC_OBSTRUCTED_UPDATE error.
If the obstructing path is the same type (file or directory) as the
corresponding path in the repository it will be left 'as-is' in the
working copy. For directories this simply means the obstruction is
tolerated. For files, any content differences between the obstruction
and the repository are treated like a local modification to the
working copy.
This patch is an expansion of one originally posted by Jonathan Gilbert
<o2...@sneakemail.com> against the 1.2.0 tag. See the various
"Takeover" threads on the dev mailing list.
* build.conf
(test-scripts): Add checkout_tests.py.
* subversion/include/svn_client.h
(svn_client_checkout3, svn_client_update3,
svn_client_switch2): New.
(svn_client_checkout2, svn_client_update2,
svn_client_switch): Deprecate.
* subversion/include/svn_wc.h
(svn_wc_notify_action_t): New 'Exists' action.
(svn_wc_get_update_editor3, svn_wc_text_modified_p2,
svn_wc_get_switch_editor3): New.
(svn_wc_get_update_editor2, svn_wc_text_modified_p,
svn_wc_get_switch_editor2): Deprecate.
* subversion/libsvn_client/checkout.c
(svn_client__checkout_internal): New argument. Update calls to
svn_client__update_internal.
(svn_client_checkout3): New.
(svn_client_checkout2, svn_client_checkout): Update calls to
svn_client__checkout_internal.
* subversion/libsvn_client/client.h
(svn_client__update_internal, svn_client__checkout_internal,
svn_client__switch_internal): New bool argument to identify updates,
checkouts, and switches, respectively, made with --force option.
* subversion/libsvn_client/copy.c
(repos_to_wc_copy): Update call to svn_client__checkout_internal.
* subversion/libsvn_client/externals.c
(switch_external): Update calls to svn_client__checkout_internal,
svn_client__update_internal, and svn_client__switch_internal.
(handle_external_item_change): Update call to
svn_client__checkout_internal.
* subversion/libsvn_client/switch.c
(svn_client__switch_internal): New argument. Replace call to
svn_wc_get_switch_editor2 with svn_wc_get_switch_editor3.
(svn_client_switch2): New.
(svn_client_switch): Update call to svn_client__switch_internal.
* subversion/libsvn_client/update.c
(svn_client__update_internal): New argument. Replace call to
svn_wc_get_update_editor2 with svn_wc_get_update_editor3.
(svn_client_update3): New.
(svn_client_update2): Reimplement as wrapper around
svn_client_update3.
(svn_client_update): Update call to svn_client__update_internal.
* subversion/libsvn_wc/adm_ops.c
(revert_admin_things): Update call to
svn_wc__text_modified_internal_p.
* subversion/libsvn_client/commit_util.c (harvest_committables):
* subversion/libsvn_wc/diff.c (file_diff, close_file):
* subversion/libsvn_wc/log.c (svn_wc_cleanup2):
* subversion/libsvn_wc/status.c (assemble_status):
Replace calls to svn_wc_text_modified_p with svn_wc_text_modified_p2.
* subversion/libsvn_wc/props.c
(svn_wc_prop_set2): Update comment.
* subversion/libsvn_wc/questions.c
(svn_wc__text_modified_internal_p): New arg. Update call to
svn_wc__text_base_path.
(svn_wc_text_modified_p2): New. Update comment.
(svn_wc_text_modified_p): Reimplement as wrapper around
svn_wc_text_modified_p2
* subversion/libsvn_wc/update_editor.c
(struct edit_baton): New member allow_unver_obstructions.
(struct file_baton): New member existed.
(make_file_baton): Initialize new file_baton member.
(add_directory): Allow obstructing directories if edit baton
permits them. Support notification for adding directories which
already 'E'xist.
(add_or_open_file): Allow obstructing files if edit baton
permits them.
(change_file_prop): Cache the last-changed-date propval for
obstructing files.
(merge_file): New argument to identify when obstructions are
allowed. Handle the various obstruction scenarios.
(close_file): Update call to merge_file. Support notification for
adding files which already 'E'xist.
(make_editor): New argument to identify when obstructions are
allowed. Initialize new edit baton member.
(svn_wc_get_update_editor3) New.
(svn_wc_get_update_editor, svn_wc_get_update_editor2): Reimplement
as wrappers around svn_wc_get_update_editor3.
(svn_wc_get_switch_editor3): New.
(svn_wc_get_switch_editor, svn_wc_get_switch_editor2): Reimplement
as wrappers around svn_wc_get_switch_editor3.
* subversion/libsvn_wc/wc.h
(svn_wc__text_modified_internal_p): New arg to select comparison
against normal text base or temporary text base.
* subversion/svn/checkout-cmd.c
(svn_cl__checkout): Replace call to svn_wc_get_update_editor2 with
svn_wc_get_update_editor3.
* subversion/svn/main.c
(svn_cl__cmd_table): Enable --force option with svn co, up, and sw.
Add new help text for each.
* subversion/svn/notify.c
(notify): Handle new 'Existed' action.
* subversion/svn/switch-cmd.c
(svn_cl__switch): Replace call to svn_client_switch with
svn_client_switch2.
* subversion/svn/update-cmd.c
(svn_cl__update): Replace call to svn_client_update2 with
svn_client_update3.
* subversion/tests/cmdline/checkout_tests.py: New.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Update to reflect new svn switch help text.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_checkout): Support optional arguments. Don't delete
WC target during forced checkouts.
* subversion/tests/cmdline/svntest/main.py
(run_and_verify_switch): Support optional arguments and regular
expression error checking.
* subversion/tests/cmdline/svntest/tree.py
(build_tree_from_checkout): Enable parsing of 'E' notification.
* subversion/tests/cmdline/switch_tests.py
(forced_switch, forced_switch_failures): New test definitions.
(test_list): Run new tests.
* subversion/tests/cmdline/update_tests.py
(forced_update, forced_update_failures): New test definitions.
(test_list): Run new tests.
]]]
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> Julian Foad <ju...@btopenworld.com> wrote on 06/22/2006 07:12:16 PM:
>>Paul Burba wrote:
>
>>>* subversion/libsvn_wc/questions.c
>>> (svn_wc__text_modified_internal_p): New arg. Update call to
>>> svn_wc__text_base_path.
>>> (svn_wc_text_modified_p2): New. Update comment.
>>> (svn_wc_text_modified_p): Reimplement as wrapper around
>>> svn_wc_text_modified_p2
>>
>>I'm not keen on adding a boolean parameter to any API. Calls with multiple
>>booleans quickly become unreadable. But it's not a big deal here
>>compared with some of our other APIs.
>>
>>Thinking out loud... I do wonder whether providing operations on thetemporary
>>text base is something we really want to be exposing in this way.
>
> Are you thinking a new function to handle comparisons with the temp text
> base is better or that operating on the temp text base is a fundamentally
> flawed design? If the latter, do you have any specific concerns?
Actually I wasn't thinking that deeply at all - it was just a "first
impression" that providing APIs to access a "temporary" object is either a bad
idea our should be done thoroughly. I do see in this patch that the vast
majority of calls to this function don't want to access the temporary copy,
which would seem to suggest that keeping an API without that option is sensible.
I'll hopefully think more about this later, and reply in another mail.
> In a nutshell, I'm using svn_wc_text_modified_p2() this way because: While
> not absolutely necessary, it's beneficial to "fix" the entries file so
> obstructing files with no actual differences have their text-time set to
> the file's last modified time. Presently the patch relies on
> svn_wc_text_modified_p() to do this, but can only do so if it can operate
> on files about to be added (i.e. whose text base exists only in the temp
> area).
>
>>Are we providing the option to access the "tmp" text base in all other
>>libsvn_wc APIs that access the text base? (I haven't looked.)
>
> No, this is something new.
>>>checkout (co): Check out a working copy from a repository.
>>>usage: checkout URL[@REV]... [PATH]
>>
>>[...]
> I see your point, how does this sound?
>
> If --force is used, unversioned obstructing paths in the working
> copy destination do not automatically cause the check out to fail.
> If the obstructing path is the same type (file or directory) as the
> corresponding path in the repository it becomes versioned but its
> contents are left 'as-is' in the working copy. This means that an
> obstructing directory's unversioned children may also obstruct and
> become versioned. For files, any content differences between the
> obstruction and the repository are treated like a local modification
> to the working copy.
That text seems good.
> Once we agree on the language I'll apply something similar to the help
> messages for sw and up.
[...]
> Sorry bout that, the bit about "applicable auto-props" is a mistake on my
> part. The patch as it stands now does *not* add any auto-props, it just
> applies the props from the repository, just as sw, up, and co normally do
> when adding files.
OK.
>>I think it would make
>>sense to give the working file the properties that "svn add" would have given
>>it, and then any differences between that set and the pristine base set from
>>the repository will show up as local modifications. To me that would be the
>>equivalent for properties of what we do with the base text, and therefore the
>>right thing to do.
>
> Regardless of the equivalency between treating props this way and how text
> is treated, I don't see it as desirable behavior. Do we really want to
> lose all the props from the repository? Is there a common use case where
> this is preferable?
What I was suggesting is that the BASE props come from the repository and the
WORKING props come from the WC's automatic rules (including "auto-props" and
auto-detection of executable, mime-type, etc.), just as if the user had "added"
or "imported" the file. I suggest that this mirrors the fact that the BASE
text comes from the repos and the WORKING text from the WC. Thus no properties
are "lost"; it is up to the user to observe any differences and decide what to
do about them, just like with the text. In some situations I feel this would
be the logical thing to do, but I'm unable to think of a good example.
The alternative, and I think this is what you are suggesting, is to set the
working props equal to the base props from the repository. I believe this
makes sense if the expected use case is when the obstructing files and
directories have been obtained from the repository and perhaps modified
somewhat, but not been replaced with different types of files (executability,
binaryness, etc.).
Now that I think about it, I agree that the latter behaviour is much more
likely to be useful in most cases, and I'm happy with it.
>>> For each item checked out a line will start with a character
>>> reporting the action taken. For unforced checkouts this character
>>> is always 'A' for 'Added'. Forced checkouts report obstructed paths
>>> with 'E' for 'Existed' and use 'A' otherwise.
>>
>>This paragraph is talking about files Added by the checkout, but doesn't say
>>so, incorrectly implying that no other codes than 'E' and 'A' can
>>result from a checkout.
>
> Talk about missing the forest for the trees, I got so caught up with
> initial checkouts into unversioned dirs that I forgot about the
> update-esque behavior of doing subsequent checkouts into an existing WC.
Heh, I can quite see how one could get into that situation with a patch of this
size ... but isn't it me getting into a muddle this time? Isn't it correct
that a checkout can only add files? It's updates (and switches) that can do
other things.
> Perhaps the best approach is to simply not mention the notification codes
> as you suggested with switch below? I've done just that for now, unless
> anyone feels strongly that 'E' warrants special mention in co's help.
If I'm right that I was wrong, put it back! Sorry for the confusion. If I'm
wrong now and was right before, please enlighten me. :-)
I think we've got the concept sorted now, and all that remains is a few
implementation details :-)
There follows a code review of about half of the patch. I've run out of time
tonight and thought it would be best to post what I have written so far.
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 20268)
> +++ subversion/include/svn_client.h (working copy)
> @@ -696,15 +696,40 @@
> * just the directory represented by @a URL and its immediate
> * non-directory children, but none of its child directories (if any).
> *
> + * If @a force_checkout is @c TRUE then checkout tolerates an existing local
> + * tree rooted at @a PATH even if some paths in the local tree are identical
This comment should probably say "unversioned" somewhere, like the other APIs
and the command-line help do.
"some paths ... are identical": The word "path" is sometimes used to mean the
item existing at a given path, so this could be misleading. I suggest making
the wording consistent with your doc strings for "update" and "switch" which
don't use the word "identical" and are clearer.
It would be really nice for the APIs to use a more descriptive parameter name
than "force" (and the "_checkout" suffix doesn't help). Think: it's quite
likely that we'll add a way to force some other aspect of its behaviour later.
Maybe use "{allow,tolerate,integrate,take_over}_unver_{items,obstructions}" ?
I don't think most of those combinations are too long given that callers
don't need to type it. Ah, I see in svn_wc_get_*_editor you've called it
"allow_obstructions". That's much better than "force", but only some kinds of
obstructions are allowed. How about using "allow_unver_obstructions"?
Whichever you choose, try to use the same name in all places except when there
is a reason not to, especially in all the public APIs that are affected.
We should seriously consider adding a more specific command-line option name
too. It's pure chance that the three commands you wanted to add this behaviour
to didn't already have "--force" options for some other purpose.
Syle nit: Personally I hate reading "is @c FALSE" and prefer "is false", and
the same with "true" and "null", as these three are well-known concepts that
don't require references to the particular preprocessor macros that we use to
represent them. In Doxygen output, of course, the "@c" is suppressed and small
capitals are used so I hardly notice it, but normally I'm reading the C header
text. However, both styles are used extensively at present so it's your
choice. (Apologies: I just can't stop myself from ranting on these sorts of
trivia!)
> + * to those in @a URL. File paths that collide will effectively treat the
> + * local file as a user modification to the pristine checkout. If @a force
> + * is @c FALSE then the checkout will abort if there are any colliding paths.
Maybe also say that working properties are set equal to the base properties?
That doesn't seem implicit to me.
> + *
> * If @a URL refers to a file rather than a directory, return the
> * error @c SVN_ERR_UNSUPPORTED_FEATURE. If @a URL does not exist,
> * return the error @c SVN_ERR_RA_ILLEGAL_URL.
> *
> * Use @a pool for any temporary allocation.
> *
> - * @since New in 1.2.
> + * @since New in 1.5.
> */
> svn_error_t *
> +svn_client_checkout3(svn_revnum_t *result_rev,
> + const char *URL,
> + const char *path,
> + const svn_opt_revision_t *peg_revision,
> + const svn_opt_revision_t *revision,
> + svn_boolean_t recurse,
> + svn_boolean_t ignore_externals,
> + svn_boolean_t force_checkout,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +
> +/**
> + * Similar to svn_client_checkout3() but with the force parameter always
Give the exact name of the parameter.
> + * set to @c FALSE.
> + *
> + * @deprecated Provided for backward compatibility with the 1.4 API.
> + */
> +svn_error_t *
> svn_client_checkout2(svn_revnum_t *result_rev,
> const char *URL,
> const char *path,
[...]
> Index: subversion/libsvn_client/externals.c
> ===================================================================
So externals won't yet support this mode. I guess that's reasonable... I've
never actually used them so have not been much interested in their semantics.
> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
[...]
> @@ -1057,15 +1066,65 @@
> || ((! copyfrom_path) && (SVN_IS_VALID_REVNUM(copyfrom_revision))))
> abort();
>
> - /* There should be nothing with this name. */
> SVN_ERR(svn_io_check_path(db->path, &kind, db->pool));
> - if (kind != svn_node_none)
> - return svn_error_createf
> - (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> - _("Failed to add directory '%s': "
> - "object of the same name already exists"),
> - svn_path_local_style(db->path, pool));
>
> + /* Check for obstructions. */
> + if (eb->allow_obstructions)
> + {
> + /* The path can exist, but it must be a directory... */
> + if (kind == svn_node_file || kind == svn_node_unknown)
"(kind != svn_node_dir)" would answer the question more directly.
> + return svn_error_createf
> + (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> + _("Failed to add directory '%s': a non-directory object of the "
> + "same name already exists"),
> + svn_path_local_style(db->path, pool));
> + else if (kind == svn_node_dir)
... and this "else if" is redundant either way.
> + {
> + /* ...Ok, it's a directory but it can't be versioned. We don't
> + handle that, yet... */
> + svn_wc_adm_access_t *adm_access;
> + const char *obstructing_path = svn_path_join(
> + pb->path, svn_path_basename(path, pool), pool);
Why is obstructing_path not just db->path? That's the path that we found of
kind svn_node_dir.
> +
> + /* Test the obstructing dir to see if it's versioned. */
> + svn_error_t *err = svn_wc_adm_open3(&adm_access, NULL,
> + obstructing_path, FALSE, 0,
> + NULL, NULL, pool);
> + if (err && err->apr_err != SVN_ERR_WC_NOT_DIRECTORY)
> + {
> + /* Something quite unexepected has happened. */
> + return err;
> + }
> + else if (err)
> + {
> + /* Obstructing dir is not versioned, just need to flag it as
> + existing then we are done here. */
> + exists = TRUE;
> + svn_error_clear(err);
> + }
> + else
> + {
> + /* Obstructing dir *is* versioned. */
> + return svn_error_createf
> + (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> + _("Failed to forcibly add directory '%s': a versioned "
> + "directory of the same name already exists"),
Minor nit: I don't think inclusion of the word "forcibly" helps, especially if
the operation was invoked through a client interface other than "svn co --force".
> + svn_path_local_style(db->path, pool));
> + }
[...]
> @@ -1684,9 +1764,9 @@
> applied. */
> fb->prop_changed = 1;
>
> - /* Special case: if the file is added during a checkout, cache the
> - last-changed-date propval for future use. */
> - if (eb->use_commit_times
> + /* Special case: if the file is added during a checkout or existed, cache
> + the last-changed-date propval for future use. */
> + if ((eb->use_commit_times || fb->existed)
Are you sure about this change? It looks odd to me, looks like the code
doesn't match the comment, even before your change. Does "if
(use_commit_times)" really mean "if (file is added during a checkout)"?
> && (strcmp(name, SVN_PROP_ENTRY_COMMITTED_DATE) == 0)
> && value)
> fb->last_changed_date = apr_pstrdup(fb->pool, value->data);
[...]
Right, that's all for tonight; I'll try to do some more but it probably won't
be tomorrow. Feel free to send an update in the meantime. Thanks for listening...
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 06/22/2006 07:12:16 PM:
> Paul Burba wrote:
> > [[[
> [...]
> > * subversion/libsvn_client/checkout.c
> > (svn_client__checkout_internal): New argument. Update calls to
> > svn_client__update_internal.
> > (svn_client_checkout3): New.
> > (svn_client_checkout): Reimplement as wrapper around
> > svn_client_checkout3.
> > (svn_client_checkout): Update call to svn_client__checkout_internal.
>
> One of those should say "(svn_client_checkout2)".
Not only that but both svn_client_checkout2 and svn_client_checkout
actually call svn_client__checkout_internal(). Fixed in the new log
below.
> > * subversion/libsvn_wc/questions.c
> > (svn_wc__text_modified_internal_p): New arg. Update call to
> > svn_wc__text_base_path.
> > (svn_wc_text_modified_p2): New. Update comment.
> > (svn_wc_text_modified_p): Reimplement as wrapper around
> > svn_wc_text_modified_p2
>
> I'm not keen on adding a boolean parameter to any API. Calls with
multiple
> booleans quickly become unreadable. But it's not a big deal here
> compared with
> some of our other APIs.
>
> Thinking out loud... I do wonder whether providing operations on
thetemporary
> text base is something we really want to be exposing in this way.
Are you thinking a new function to handle comparisons with the temp text
base is better or that operating on the temp text base is a fundamentally
flawed design? If the latter, do you have any specific concerns?
In a nutshell, I'm using svn_wc_text_modified_p2() this way because: While
not absolutely necessary, it's beneficial to "fix" the entries file so
obstructing files with no actual differences have their text-time set to
the file's last modified time. Presently the patch relies on
svn_wc_text_modified_p() to do this, but can only do so if it can operate
on files about to be added (i.e. whose text base exists only in the temp
area).
> Are we
> providing the option to access the "tmp" text base in all other
> libsvn_wc APIs
> that access the text base? (I haven't looked.)
No, this is something new.
> > * subversion/libsvn_wc/wc.h
> > (svn_wc__text_modified_internal_p): New arg.
>
> It would be helpful if the log indicated why or what the new arg is,like
you
> have done with other items, so I could get a feel for the change
> without going
> to the code.
Done.
> > Index: subversion/include/svn_wc.h
> > ===================================================================
> [...]
> > +/* Similar to svn_wc_text_modified_p2() but with the use_tmp_base
>
> Start Doxygen comments with "/**". Doxygen says:
>
> subversion/include/svn_wc.h:1125: Warning: Member
> svn_wc_text_modified_p... is
> not documented.
> subversion/include/svn_wc.h:2537: Warning: Member
> svn_wc_get_update_editor2...
> is not documented.
> subversion/include/svn_wc.h:2638: Warning: Member
> svn_wc_get_switch_editor2...
> is not documented.
Fixed all.
> > checkout (co): Check out a working copy from a repository.
> > usage: checkout URL[@REV]... [PATH]
> [...]
> > If --force is used, unversioned obstructing paths in the working
> > copy destination do not automatically cause the check out to fail.
> > If the obstructing path is the same type (file or directory) as the
> > corresponding path in the repository it will be left 'as-is' in the
> > working copy. For directories this simply means the obstruction is
>
> "simply ... the obstruction is tolerated": this implies to me that
> nothing will
> be done to the unversioned directory. However, "svn checkout"
> converts it into
> a versioned directory and then proceeds inside it. Perhaps you
werethinking
> of the implementation step that tries to create the directory tolerating
its
> existence; however, from the user's point of view of the whole
> operation, it is
> not simply tolerated but is converted.
I see your point, how does this sound?
If --force is used, unversioned obstructing paths in the working
copy destination do not automatically cause the check out to fail.
If the obstructing path is the same type (file or directory) as the
corresponding path in the repository it becomes versioned but its
contents are left 'as-is' in the working copy. This means that an
obstructing directory's unversioned children may also obstruct and
become versioned. For files, any content differences between the
obstruction and the repository are treated like a local modification
to the working copy.
Once we agree on the language I'll apply something similar to the help
messages for sw and up.
> > tolerated. For files, any content differences between the
> > obstruction and the repository are treated like a local modification
> > to the working copy. All properties from the repository and
> > applicable auto-props are applied to the obstructing path.
>
> Properties from the repository AND those that would have been given to
it by
> "avn add"? As in the two sets of properties are merged?
Sorry bout that, the bit about "applicable auto-props" is a mistake on my
part. The patch as it stands now does *not* add any auto-props, it just
applies the props from the repository, just as sw, up, and co normally do
when adding files.
> I think it would make
> sense to give the working file the properties that "svn add" would have
given
> it, and then any differences between that set and the pristine base set
from
> the repository will show up as local modifications. To me that would be
the
> equivalent for properties of what we do with the base text, and
therefore the
> right thing to do.
Regardless of the equivalency between treating props this way and how text
is treated, I don't see it as desirable behavior. Do we really want to
lose all the props from the repository? Is there a common use case where
this is preferable?
> (I may have missed some design decisions, or maybe it just wasn't
clearly
> resolved.)
It's not so much that the question of props was not clearly resolved, it
wasn't really discussed that I can recall, it just fell through the
cracks.
> > For each item checked out a line will start with a character
> > reporting the action taken. For unforced checkouts this character
> > is always 'A' for 'Added'. Forced checkouts report obstructed paths
> > with 'E' for 'Existed' and use 'A' otherwise.
>
> This paragraph is talking about files Added by the checkout, but doesn't
say
> so, incorrectly implying that no other codes than 'E' and 'A' can
> result from a
> checkout.
Talk about missing the forest for the trees, I got so caught up with
initial checkouts into unversioned dirs that I forgot about the
update-esque behavior of doing subsequent checkouts into an existing WC.
Perhaps the best approach is to simply not mention the notification codes
as you suggested with switch below? I've done just that for now, unless
anyone feels strongly that 'E' warrants special mention in co's help.
> > update (up): Bring changes from the repository into the working copy.
> > usage: update [PATH...]
> [...]
> > For each updated item a line will start with a character reporting
the
> > action taken. These characters have the following meaning:
> >
> > A Added
> > D Deleted
> > U Updated
> > C Conflict
> > G Merged
> >
> > A character in the first column signifies an update to the actual
file,
> [...]
> >
> > If --force is used, unversioned obstructing paths in the working
> [...]
> > auto-props are applied to the obstructing path. Obstructing paths
are
> > reported in the first column with:
> >
> > E Existed
>
> I would suggest this notification code belongs in the list above, and
just
> ending this last sentence with "with the code 'E'."
That's reasonable, done.
> > switch (sw): Update the working copy to a different URL.
> > usage: 1. switch URL [PATH]
> [...]
> > auto-props are applied to the obstructing path. Obstructing paths
are
> > reported in the first column with:
> >
> > E Existed
>
> This last sentence doesn't make so much sense in the "switch" command
help
> because columns and the codes used in them haven't been mentioned
> earlier on in
> this help like they are in "update" help. It is probably best to omit
this
> sentence entirely, and let the user find the full list and partial
> description
> of the notifications in the help for "update".
Ditto.
> Otherwise, the log looks good. I don't have time to check the code
> thoroughly,
> but I'm building it so I can try it out. I have recently been
> wanting this at
> work so I'm glad you're doing this.
>
> - Julian
Thanks for taking the time to look this over. Please let me know your
thoughts on the open issues above.
Paul B.
[[[
Support --force option with svn checkout, update, and switch.
With the force option, co, sw, and up now tolerate unversioned
obstructing paths when adding new paths of the same type, rather than
generating a SVN_ERR_WC_OBSTRUCTED_UPDATE error.
If the obstructing path is the same type (file or directory) as the
corresponding path in the repository it will be left 'as-is' in the
working copy. For directories this simply means the obstruction is
tolerated. For files, any content differences between the obstruction
and the repository are treated like a local modification to the
working copy.
This patch is an expansion of one originally posted by Jonathan Gilbert
<o2...@sneakemail.com> against the 1.2.0 tag. See the various
"Takeover" threads on the dev mailing list.
* build.conf
(test-scripts): Add checkout_tests.py.
* subversion/include/svn_client.h
(svn_client_checkout3, svn_client_update3,
svn_client_switch2): New.
(svn_client_checkout2, svn_client_update2,
svn_client_switch): Deprecate.
* subversion/include/svn_wc.h
(svn_wc_notify_action_t): New 'Exists' action.
(svn_wc_get_update_editor3, svn_wc_text_modified_p2,
svn_wc_get_switch_editor3): New.
(svn_wc_get_update_editor2, svn_wc_text_modified_p,
svn_wc_get_switch_editor2): Deprecate.
* subversion/libsvn_client/checkout.c
(svn_client__checkout_internal): New argument. Update calls to
svn_client__update_internal.
(svn_client_checkout3): New.
(svn_client_checkout2, svn_client_checkout): Update calls to
svn_client__checkout_internal.
* subversion/libsvn_client/client.h
(svn_client__update_internal, svn_client__checkout_internal,
svn_client__switch_internal): New bool argument to identify updates,
checkouts, and switches, respectively, made with --force option.
* subversion/libsvn_client/copy.c
(repos_to_wc_copy): Update call to svn_client__checkout_internal.
* subversion/libsvn_client/externals.c
(switch_external): Update calls to svn_client__checkout_internal,
svn_client__update_internal, and svn_client__switch_internal.
(handle_external_item_change): Update call to
svn_client__checkout_internal.
* subversion/libsvn_client/switch.c
(svn_client__switch_internal): New argument. Replace call to
svn_wc_get_switch_editor2 with svn_wc_get_switch_editor3.
(svn_client_switch2): New.
(svn_client_switch): Update call to svn_client__switch_internal.
* subversion/libsvn_client/update.c
(svn_client__update_internal): New argument. Replace call to
svn_wc_get_update_editor2 with svn_wc_get_update_editor3.
(svn_client_update3): New.
(svn_client_update2): Reimplement as wrapper around
svn_client_update3.
(svn_client_update): Update call to svn_client__update_internal.
* subversion/libsvn_wc/adm_ops.c
(revert_admin_things): Update call to
svn_wc__text_modified_internal_p.
* subversion/libsvn_client/commit_util.c (harvest_committables):
* subversion/libsvn_wc/diff.c (file_diff, close_file):
* subversion/libsvn_wc/log.c (svn_wc_cleanup2):
* subversion/libsvn_wc/status.c (assemble_status):
Replace calls to svn_wc_text_modified_p with svn_wc_text_modified_p2.
* subversion/libsvn_wc/props.c
(svn_wc_prop_set2): Update comment.
* subversion/libsvn_wc/questions.c
(svn_wc__text_modified_internal_p): New arg. Update call to
svn_wc__text_base_path.
(svn_wc_text_modified_p2): New. Update comment.
(svn_wc_text_modified_p): Reimplement as wrapper around
svn_wc_text_modified_p2
* subversion/libsvn_wc/update_editor.c
(struct edit_baton): New member allow_obstructions.
(struct file_baton): New member existed.
(make_file_baton): Initialize new file_baton member.
(add_directory): Allow obstructing directories if edit baton
permits them. Support notification for adding directories which
already 'E'xist.
(add_or_open_file): Allow obstructing files if edit baton
permits them.
(change_file_prop): Cache the last-changed-date propval for
obstructing files.
(merge_file): New argument to identify when obstructions are
allowed. Handle the various obstruction scenarios.
(close_file): Update call to merge_file. Support notification for
adding files which already 'E'xist.
(make_editor): New argument to identify when obstructions are
allowed. Initialize new edit baton member.
(svn_wc_get_update_editor3) New.
(svn_wc_get_update_editor, svn_wc_get_update_editor2): Reimplement
as wrappers around svn_wc_get_update_editor3.
(svn_wc_get_switch_editor3): New.
(svn_wc_get_switch_editor, svn_wc_get_switch_editor2): Reimplement
as wrappers around svn_wc_get_switch_editor3.
* subversion/libsvn_wc/wc.h
(svn_wc__text_modified_internal_p): New arg to select comparison
against normal text base or temporary text base.
* subversion/svn/checkout-cmd.c
(svn_cl__checkout): Replace call to svn_wc_get_update_editor2 with
svn_wc_get_update_editor3.
* subversion/svn/main.c
(svn_cl__cmd_table): Enable --force option with svn co, up, and sw.
Add new help text for each.
* subversion/svn/notify.c
(notify): Handle new 'Existed' action.
* subversion/svn/switch-cmd.c
(svn_cl__switch): Replace call to svn_client_switch with
svn_client_switch2.
* subversion/svn/update-cmd.c
(svn_cl__update): Replace call to svn_client_update2 with
svn_client_update3.
* subversion/tests/cmdline/checkout_tests.py: New.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Update to reflect new svn switch help text.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_checkout): Support optional arguments. Don't delete
WC target during forced checkouts.
* subversion/tests/cmdline/svntest/main.py
(run_and_verify_switch): Support optional arguments and regular
expression error checking.
* subversion/tests/cmdline/svntest/tree.py
(build_tree_from_checkout): Enable parsing of 'E' notification.
* subversion/tests/cmdline/switch_tests.py
(forced_switch, forced_switch_failures): New test definitions.
(test_list): Run new tests.
* subversion/tests/cmdline/update_tests.py
(forced_update, forced_update_failures): New test definitions.
(test_list): Run new tests.
]]]
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Paul Burba <pa...@softlanding.com>.
Julian Foad <ju...@btopenworld.com> wrote on 06/22/2006 07:12:16 PM:
> Paul Burba wrote:
> >
> > Sidebar: I noticed that this "repair" behavior is not described in the
doc
> > string for svn_wc_text_modified_p(), should it be?
>
> Yes, I'd say that is worth mentioning. The way to document it wouldbe
to say
> that the function *may* repair the time stamp, not that it *does*.
> If that was
> already the case, a separate patch would be ideal but I'd be happy
> for it to be
> included in this patch if you like.
Julian,
This repair behavior existed prior to my patch.
I updated the doc string for svn_wc_text_modified_p() in r20249.
Paul B.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> 1) "Takeover" is no more, the term that is. [...]
Good. All other changes good.
> Sidebar: I noticed that this "repair" behavior is not described in the doc
> string for svn_wc_text_modified_p(), should it be?
Yes, I'd say that is worth mentioning. The way to document it would be to say
that the function *may* repair the time stamp, not that it *does*. If that was
already the case, a separate patch would be ideal but I'd be happy for it to be
included in this patch if you like.
> [[[
[...]
> * subversion/libsvn_client/checkout.c
> (svn_client__checkout_internal): New argument. Update calls to
> svn_client__update_internal.
> (svn_client_checkout3): New.
> (svn_client_checkout): Reimplement as wrapper around
> svn_client_checkout3.
> (svn_client_checkout): Update call to svn_client__checkout_internal.
One of those should say "(svn_client_checkout2)".
> * subversion/libsvn_wc/questions.c
> (svn_wc__text_modified_internal_p): New arg. Update call to
> svn_wc__text_base_path.
> (svn_wc_text_modified_p2): New. Update comment.
> (svn_wc_text_modified_p): Reimplement as wrapper around
> svn_wc_text_modified_p2
I'm not keen on adding a boolean parameter to any API. Calls with multiple
booleans quickly become unreadable. But it's not a big deal here compared with
some of our other APIs.
Thinking out loud... I do wonder whether providing operations on the temporary
text base is something we really want to be exposing in this way. Are we
providing the option to access the "tmp" text base in all other libsvn_wc APIs
that access the text base? (I haven't looked.)
> * subversion/libsvn_wc/wc.h
> (svn_wc__text_modified_internal_p): New arg.
It would be helpful if the log indicated why or what the new arg is, like you
have done with other items, so I could get a feel for the change without going
to the code.
> Index: subversion/include/svn_wc.h
> ===================================================================
[...]
> +/* Similar to svn_wc_text_modified_p2() but with the use_tmp_base
Start Doxygen comments with "/**". Doxygen says:
subversion/include/svn_wc.h:1125: Warning: Member svn_wc_text_modified_p... is
not documented.
subversion/include/svn_wc.h:2537: Warning: Member svn_wc_get_update_editor2...
is not documented.
subversion/include/svn_wc.h:2638: Warning: Member svn_wc_get_switch_editor2...
is not documented.
> checkout (co): Check out a working copy from a repository.
> usage: checkout URL[@REV]... [PATH]
[...]
> If --force is used, unversioned obstructing paths in the working
> copy destination do not automatically cause the check out to fail.
> If the obstructing path is the same type (file or directory) as the
> corresponding path in the repository it will be left 'as-is' in the
> working copy. For directories this simply means the obstruction is
"simply ... the obstruction is tolerated": this implies to me that nothing will
be done to the unversioned directory. However, "svn checkout" converts it into
a versioned directory and then proceeds inside it. Perhaps you were thinking
of the implementation step that tries to create the directory tolerating its
existence; however, from the user's point of view of the whole operation, it is
not simply tolerated but is converted.
> tolerated. For files, any content differences between the
> obstruction and the repository are treated like a local modification
> to the working copy. All properties from the repository and
> applicable auto-props are applied to the obstructing path.
Properties from the repository AND those that would have been given to it by
"avn add"? As in the two sets of properties are merged? I think it would make
sense to give the working file the properties that "svn add" would have given
it, and then any differences between that set and the pristine base set from
the repository will show up as local modifications. To me that would be the
equivalent for properties of what we do with the base text, and therefore the
right thing to do.
(I may have missed some design decisions, or maybe it just wasn't clearly
resolved.)
> For each item checked out a line will start with a character
> reporting the action taken. For unforced checkouts this character
> is always 'A' for 'Added'. Forced checkouts report obstructed paths
> with 'E' for 'Existed' and use 'A' otherwise.
This paragraph is talking about files Added by the checkout, but doesn't say
so, incorrectly implying that no other codes than 'E' and 'A' can result from a
checkout.
> update (up): Bring changes from the repository into the working copy.
> usage: update [PATH...]
[...]
> For each updated item a line will start with a character reporting the
> action taken. These characters have the following meaning:
>
> A Added
> D Deleted
> U Updated
> C Conflict
> G Merged
>
> A character in the first column signifies an update to the actual file,
[...]
>
> If --force is used, unversioned obstructing paths in the working
[...]
> auto-props are applied to the obstructing path. Obstructing paths are
> reported in the first column with:
>
> E Existed
I would suggest this notification code belongs in the list above, and just
ending this last sentence with "with the code 'E'."
> switch (sw): Update the working copy to a different URL.
> usage: 1. switch URL [PATH]
[...]
> auto-props are applied to the obstructing path. Obstructing paths are
> reported in the first column with:
>
> E Existed
This last sentence doesn't make so much sense in the "switch" command help
because columns and the codes used in them haven't been mentioned earlier on in
this help like they are in "update" help. It is probably best to omit this
sentence entirely, and let the user find the full list and partial description
of the notifications in the help for "update".
Otherwise, the log looks good. I don't have time to check the code thoroughly,
but I'm building it so I can try it out. I have recently been wanting this at
work so I'm glad you're doing this.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Was [PROPOSAL] Takeover Take 2
Posted by Paul Burba <pa...@softlanding.com>.
Hi All,
I'm reposting this patch in the hope someone might be able to take a look.
It is a fairly complex change compared to what I've done before (at least
outside of the OS400/EBCDIC port) so I'd like to have another set of eyes
look at it.
Thanks,
Paul B.
========================================
Paul Burba <pa...@softlanding.com> wrote on 06/15/2006 02:01:30 PM:
Hi All,
After a delay working on some other things I had a chance to rework this
patch based on your various suggestions. While there are a lot of little
tweaks (e.g. improved comments, new Python tests) the following are the
major changes from the previous patch:
----------------------------------------
1) "Takeover" is no more, the term that is. This patch is about
tolerating obstructions to added items, so to avoid confusion I've removed
all reference to "takeover" in the patch. This doesn't actually have much
of a user-visible impact, just that the notification of obstructions is no
longer 'T'akeover but rather 'E' for 'already Exists' (Thanks Philip).
----------------------------------------
2) Obstruction tolerance is now available using --force for update and
switch as well as checkout. The --help messages for sw, up, and co
describe the new behavior.
----------------------------------------
3) Obstructing versioned directory obstructions are no longer allowed and
always cause an error.
----------------------------------------
4) svn_client_cleanup() is no longer used to "fix" the entries file so
that 'E'xisting WC files with no actual modifications have their text-time
field set the file's last modified time. Instead I revved
svn_wc_text_modified_p() so it has the option to use the base revision in
tmp when checking for mods. Now we can not only check if an obstructing
file differs from a file about to be added, but svn_wc_text_modified_p()
cleans up the entries file:
From svn_wc__text_modified_internal_p():
/* It is quite legitimate for modifications to the working copy to
produce a timestamp variation with no text variation. If it turns out
that there are no differences then we might be able to "repair" the
text-time in the entries file and so avoid the expensive file
contents
comparison in the future. */
if (! *modified_p && svn_wc_adm_locked(adm_access))
{
svn_wc_entry_t tmp;
SVN_ERR(svn_io_file_affected_time(&tmp.text_time, filename, pool));
SVN_ERR(svn_wc__entry_modify(adm_access,
svn_path_basename(filename, pool),
&tmp, SVN_WC__ENTRY_MODIFY_TEXT_TIME,
TRUE,
pool));
}
Sidebar: I noticed that this "repair" behavior is not described in the doc
string for svn_wc_text_modified_p(), should it be?
----------------------------------------
5) Eliminated references to the command line client in the svn_wc_* APIs
and code.
----------------------------------------
Please take a look if you have some time. Any and all feedback
appreciated, thanks,
Paul B.
[[[
Support --force option with svn checkout, update, and switch.
With the force option, co, sw, and up now tolerate unversioned
obstructing paths when adding new paths of the same type, rather than
generating a SVN_ERR_WC_OBSTRUCTED_UPDATE error.
If the obstructing path is the same type (file or directory) as the
corresponding path in the repository it will be left 'as-is' in the
working copy. For directories this simply means the obstruction is
tolerated. For files, any content differences between the obstruction
and the repository are treated like a local modification to the
working copy.
This patch is an expansion of one originally posted by Jonathan Gilbert
<o2...@sneakemail.com> against the 1.2.0 tag. See the various
"Takeover" threads on the dev mailing list.
* build.conf
(test-scripts): Add checkout_tests.py.
* subversion/include/svn_client.h
(svn_client_checkout3, svn_client_update3,
svn_client_switch2): New.
(svn_client_checkout2, svn_client_update2,
svn_client_switch): Deprecate.
* subversion/include/svn_wc.h
(svn_wc_notify_action_t): New 'Exists' action.
(svn_wc_get_update_editor3, svn_wc_text_modified_p2,
svn_wc_get_switch_editor3): New.
(svn_wc_get_update_editor2, svn_wc_text_modified_p,
svn_wc_get_switch_editor2): Deprecate.
* subversion/libsvn_client/checkout.c
(svn_client__checkout_internal): New argument. Update calls to
svn_client__update_internal.
(svn_client_checkout3): New.
(svn_client_checkout): Reimplement as wrapper around
svn_client_checkout3.
(svn_client_checkout): Update call to svn_client__checkout_internal.
* subversion/libsvn_client/client.h
(svn_client__update_internal, svn_client__checkout_internal,
svn_client__switch_internal): New bool argument to identify updates,
checkouts, and switches, respectively, made with --force option.
* subversion/libsvn_client/copy.c
(repos_to_wc_copy): Update call to svn_client__checkout_internal.
* subversion/libsvn_client/externals.c
(switch_external): Update calls to svn_client__checkout_internal,
svn_client__update_internal, and svn_client__switch_internal.
(handle_external_item_change): Update call to
svn_client__checkout_internal.
* subversion/libsvn_client/switch.c
(svn_client__switch_internal): New argument. Replace call to
svn_wc_get_switch_editor2 with svn_wc_get_switch_editor3.
(svn_client_switch2): New.
(svn_client_switch): Update call to svn_client__switch_internal.
* subversion/libsvn_client/update.c
(svn_client__update_internal): New argument. Replace call to
svn_wc_get_update_editor2 with svn_wc_get_update_editor3.
(svn_client_update3): New.
(svn_client_update2): Reimplement as wrapper around
svn_client_update3.
(svn_client_update): Update call to svn_client__update_internal.
* subversion/libsvn_wc/adm_ops.c
(revert_admin_things): Update call to
svn_wc__text_modified_internal_p.
* subversion/libsvn_client/commit_util.c (harvest_committables):
* subversion/libsvn_wc/diff.c (file_diff, close_file):
* subversion/libsvn_wc/log.c (svn_wc_cleanup2):
* subversion/libsvn_wc/status.c (assemble_status):
Replace calls to svn_wc_text_modified_p with svn_wc_text_modified_p2.
* subversion/libsvn_wc/props.c
(svn_wc_prop_set2): Update comment.
* subversion/libsvn_wc/questions.c
(svn_wc__text_modified_internal_p): New arg. Update call to
svn_wc__text_base_path.
(svn_wc_text_modified_p2): New. Update comment.
(svn_wc_text_modified_p): Reimplement as wrapper around
svn_wc_text_modified_p2
* subversion/libsvn_wc/update_editor.c
(struct edit_baton): New member allow_obstructions.
(struct file_baton): New member existed.
(make_file_baton): Initialize new file_baton member.
(add_directory): Allow obstructing directories if edit baton
permits them. Support notification for adding directories which
already 'E'xist.
(add_or_open_file): Allow obstructing files if edit baton
permits them.
(change_file_prop): Cache the last-changed-date propval for
obstructing files.
(merge_file): New argument to identify when obstructions are
allowed. Handle the various obstruction scenarios.
(close_file): Update call to merge_file. Support notification for
adding files which already 'E'xist.
(make_editor): New argument to identify when obstructions are
allowed. Initialize new edit baton member.
(svn_wc_get_update_editor3) New.
(svn_wc_get_update_editor, svn_wc_get_update_editor2): Reimplement
as wrappers around svn_wc_get_update_editor3.
(svn_wc_get_switch_editor3): New.
(svn_wc_get_switch_editor, svn_wc_get_switch_editor2): Reimplement
as wrappers around svn_wc_get_switch_editor3.
* subversion/libsvn_wc/wc.h
(svn_wc__text_modified_internal_p): New arg.
* subversion/svn/checkout-cmd.c
(svn_cl__checkout): Replace call to svn_wc_get_update_editor2 with
svn_wc_get_update_editor3.
* subversion/svn/main.c
(svn_cl__cmd_table): Enable --force option with svn co, up, and sw.
Add new help text for each.
* subversion/svn/notify.c
(notify): Handle new 'Existed' action.
* subversion/svn/switch-cmd.c
(svn_cl__switch): Replace call to svn_client_switch with
svn_client_switch2.
* subversion/svn/update-cmd.c
(svn_cl__update): Replace call to svn_client_update2 with
svn_client_update3.
* subversion/tests/cmdline/checkout_tests.py: New.
* subversion/tests/cmdline/getopt_tests_data/svn_help_log_switch_stdout
Update to reflect new svn switch help text.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_checkout): Support optional arguments. Don't delete
WC target during forced checkouts.
* subversion/tests/cmdline/svntest/main.py
(run_and_verify_switch): Support optional arguments and regular
expression error checking.
* subversion/tests/cmdline/svntest/tree.py
(build_tree_from_checkout): Enable parsing of 'E' notification.
* subversion/tests/cmdline/switch_tests.py
(forced_switch, forced_switch_failures): New test definitions.
(test_list): Run new tests.
* subversion/tests/cmdline/update_tests.py
(forced_update, forced_update_failures): New test definitions.
(test_list): Run new tests.
]]]