Julian Foad <julianfoad@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
<o2w9gs702@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.
]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jun 27 21:21:12 2006