You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/04/08 21:02:00 UTC

deprecate all of svn_wc.h ?

Hey all,

It has come up a few times on IRC discussions: "we should never have
exposed libsvn_wc, just libsvn_client".

Well, we've already exposed it, so we need to at least keep that stuff
around. But moving forward... should new functions continue to be
exposed? Or should all new functions go into svn_wc_private.h?

This question is (probably) directed at our (GUI) client developers.
Do you ever use WC functions? And if you do, then which ones? Where is
svn_client.h insufficient, leading you to use svn_wc.h APIs?

Note: I think the separation is a Good Thing, for our benefit, but we
don't necessarily have to expose the WC layer to downstream
developers.

Cheers,
-g

Re: deprecate all of svn_wc.h ?

Posted by Hyrum Wright <hy...@hyrumwright.org>.
On Thu, Apr 8, 2010 at 10:02 PM, Greg Stein <gs...@gmail.com> wrote:

> Hey all,
>
> It has come up a few times on IRC discussions: "we should never have
> exposed libsvn_wc, just libsvn_client".
>
> Well, we've already exposed it, so we need to at least keep that stuff
> around. But moving forward... should new functions continue to be
> exposed? Or should all new functions go into svn_wc_private.h?
>
> This question is (probably) directed at our (GUI) client developers.
> Do you ever use WC functions? And if you do, then which ones? Where is
> svn_client.h insufficient, leading you to use svn_wc.h APIs?
>
> Note: I think the separation is a Good Thing, for our benefit, but we
> don't necessarily have to expose the WC layer to downstream
> developers.
>

I am not a GUI client developer.

That being said: I don't think we should deprecate svn_wc.h until 2.0.
Don't get me wrong: I would like to dump all that stuff and not have to
worry about holding on to backward compat for new wc APIs going forward.
I'm just concerned that junking stuff now would result in mass confusion
when rev'ing stuff in svn_wc.h.  For instance, all the notification types
are defined as svn_wc_notify_*.  If we deprecate svn_wc.h, what happens when
we need to add another notification type in 1.8?  It seems like a lot of
work for whoever that unfortunate soul happens to be.

You rightly point out that things are a mess in the interface between
libsvn_client and libsvn_wc right now.  To really fix the problem, we need
to examine the interfaces, and move stuff there it more appropriately
belongs (e.g., notification should be it's own subsystem, *not* part of
libsvn_wc).  Whenever 2.0 happens, it will give us the chance to do that,
but fulling deprecating svn_wc.h right now is just a band-aid over a much
deeper problem, and will make things even worse.

-Hyrum

Re: deprecate all of svn_wc.h ?

Posted by Barry Scott <ba...@barrys-emacs.org>.
On 16 May 2010, at 16:20, Greg Stein wrote:

> On Sun, May 16, 2010 at 05:00, Barry Scott <ba...@barrys-emacs.org> wrote:
>> ...
>> Here are the list of svn_wc_ symbols from pysvn.
>> pysvn does not include svn_wc.h.
> 
> How do you have these values if you don't include svn_wc.h?
> 
> I think you *implicitly* include it through your inclusion of svn_client.h.

I do include svn_client.h and it does include wvn_wc.h so that will be it.

I try to only use the svn_client_ interface but that requires use of svn_wc_
symbols for operating the exposed API. LIke with notifications.

Barry

Re: deprecate all of svn_wc.h ?

Posted by Greg Stein <gs...@gmail.com>.
On Sun, May 16, 2010 at 05:00, Barry Scott <ba...@barrys-emacs.org> wrote:
>...
> Here are the list of svn_wc_ symbols from pysvn.
> pysvn does not include svn_wc.h.

How do you have these values if you don't include svn_wc.h?

I think you *implicitly* include it through your inclusion of svn_client.h.

Thx,
-g

Re: deprecate all of svn_wc.h ?

Posted by Barry Scott <ba...@barrys-emacs.org>.
On 8 Apr 2010, at 22:02, Greg Stein wrote:

> Hey all,
> 
> It has come up a few times on IRC discussions: "we should never have
> exposed libsvn_wc, just libsvn_client".
> 
> Well, we've already exposed it, so we need to at least keep that stuff
> around. But moving forward... should new functions continue to be
> exposed? Or should all new functions go into svn_wc_private.h?
> 
> This question is (probably) directed at our (GUI) client developers.
> Do you ever use WC functions? And if you do, then which ones? Where is
> svn_client.h insufficient, leading you to use svn_wc.h APIs?
> 
> Note: I think the separation is a Good Thing, for our benefit, but we
> don't necessarily have to expose the WC layer to downstream
> developers.
> 
> Cheers,
> -g

Here are the list of svn_wc_ symbols from pysvn.
pysvn does not include svn_wc.h.

svn_wc_adm_access_t
svn_wc_adm_probe_open
svn_wc_adm_probe_open3
svn_wc_conflict_action_add
svn_wc_conflict_action_delete
svn_wc_conflict_action_edit
svn_wc_conflict_action_t
svn_wc_conflict_choice_t
svn_wc_conflict_choose_base
svn_wc_conflict_choose_merged
svn_wc_conflict_choose_mine_conflict
svn_wc_conflict_choose_mine_full
svn_wc_conflict_choose_postpone
svn_wc_conflict_choose_theirs_conflict
svn_wc_conflict_choose_theirs_full
svn_wc_conflict_description_t
svn_wc_conflict_kind_property
svn_wc_conflict_kind_t
svn_wc_conflict_kind_text
svn_wc_conflict_reason_deleted
svn_wc_conflict_reason_edited
svn_wc_conflict_reason_missing
svn_wc_conflict_reason_obstructed
svn_wc_conflict_reason_t
svn_wc_conflict_reason_unversioned
svn_wc_conflict_result_t
svn_wc_conflict_version_t
svn_wc_create_conflict_result
svn_wc_dup_status
svn_wc_dup_status2
svn_wc_entry
svn_wc_entry_t
svn_wc_get_adm_dir
svn_wc_info_dup
svn_wc_info_t
svn_wc_is_adm_dir
svn_wc_merge_conflict
svn_wc_merge_merged
svn_wc_merge_no_merge
svn_wc_merge_outcome_t
svn_wc_merge_unchanged
svn_wc_notify_action_t
svn_wc_notify_add
svn_wc_notify_blame_revision
svn_wc_notify_changelist_clear
svn_wc_notify_changelist_moved
svn_wc_notify_changelist_set
svn_wc_notify_commit_added
svn_wc_notify_commit_deleted
svn_wc_notify_commit_modified
svn_wc_notify_commit_postfix_txdelta
svn_wc_notify_commit_replaced
svn_wc_notify_copy
svn_wc_notify_delete
svn_wc_notify_exists
svn_wc_notify_failed_external
svn_wc_notify_failed_lock
svn_wc_notify_failed_revert
svn_wc_notify_failed_unlock
svn_wc_notify_foreign_merge_begin
svn_wc_notify_locked
svn_wc_notify_merge_begin
svn_wc_notify_merge_completed
svn_wc_notify_property_added
svn_wc_notify_property_deleted
svn_wc_notify_property_deleted_nonexistent
svn_wc_notify_property_modified
svn_wc_notify_resolved
svn_wc_notify_restore
svn_wc_notify_revert
svn_wc_notify_revprop_deleted
svn_wc_notify_revprop_set
svn_wc_notify_skip
svn_wc_notify_state_changed
svn_wc_notify_state_conflicted
svn_wc_notify_state_inapplicable
svn_wc_notify_state_merged
svn_wc_notify_state_missing
svn_wc_notify_state_obstructed
svn_wc_notify_state_t
svn_wc_notify_state_unchanged
svn_wc_notify_state_unknown
svn_wc_notify_status_completed
svn_wc_notify_status_external
svn_wc_notify_t
svn_wc_notify_tree_conflict
svn_wc_notify_unlocked
svn_wc_notify_update_add
svn_wc_notify_update_completed
svn_wc_notify_update_delete
svn_wc_notify_update_external
svn_wc_notify_update_replace
svn_wc_notify_update_update
svn_wc_operation_merge
svn_wc_operation_none
svn_wc_operation_switch
svn_wc_operation_t
svn_wc_operation_update
svn_wc_schedule_add
svn_wc_schedule_delete
svn_wc_schedule_normal
svn_wc_schedule_replace
svn_wc_schedule_t
svn_wc_set_adm_dir
svn_wc_status2_t
svn_wc_status_added
svn_wc_status_conflicted
svn_wc_status_deleted
svn_wc_status_external
svn_wc_status_ignored
svn_wc_status_incomplete
svn_wc_status_kind
svn_wc_status_merged
svn_wc_status_missing
svn_wc_status_modified
svn_wc_status_none
svn_wc_status_normal
svn_wc_status_obstructed
svn_wc_status_replaced
svn_wc_status_t
svn_wc_status_unversioned

Barry

Re: deprecate all of svn_wc.h ?

Posted by Stefan Küng <to...@gmail.com>.
On 09.04.2010 21:07, Greg Stein wrote:
> On Fri, Apr 9, 2010 at 13:49, Stefan Küng<to...@gmail.com>  wrote:
>> ...
>> Going through the code of TSVN, I found a few places where we use functions
>> defined in svn_wc.h:
>
> Ooh! Thanks!
>
>> * svn_wc_adm_probe_open3, used to get the svn_wc_adm_access_t required for
>> svn_client_uuid_from_path()
>
> Looks like we have an access-baton-free replacement
> (svn_client_uuid_from_path2).

Great! I was wondering why an svn_client_ function would need an access 
baton anyway.

>
>> * svn_wc_translated_file2
>> * svn_wc_prop_list
>> * svn_wc_is_adm_dir
>
> These should be fine.
>
>> * svn_wc_dup_status2
>
> I think we're going to be revamping the status structure pretty hard. It may

I'm aware that the status functions and structures will change a lot.

>> * svn_wc_get_pristine_copy_path (we don't use svn_wc_get_pristine_contents
>> because that returns a stream, and we need a file)
>
> Ugh. This one is particularly problematic.
>
> Can I ask what you use it for, so that we can come up with a suitable
> replacement?

We use it to get the pristine file when we do diffs WC->BASE. For 
example, users can doubleclick on modified files in most dialogs 
(especially the commit dialog) to show the diff before committing.

I know that there's svn_wc_get_pristine_contents which we should use 
instead, but we need a file, not a stream to work with (the file is 
needed because we start another app to do the diff). Sure, we could use 
the stream and save that ourselves to a file instead, but that's what 
svn_wc_get_pristine_copy_path already does, so we still use that api.

>
>> * svn_wc_set_adm_dir
>
> Set? Is this for when you detect a working copy that uses _svn ?

This is needed. Check the svn main.c file (for 1.6.x at least). There's 
this in there:

#if defined(WIN32) || defined(__CYGWIN__)
   /* Set the working copy administrative directory name. */
   if (getenv("SVN_ASP_DOT_NET_HACK"))
     {
       err = svn_wc_set_adm_dir("_svn", pool);
       if (err)
         return svn_cmdline_handle_exit_error(err, pool, "svn: ");
     }
#endif

So if the command line client has to initialize this, we have to do it too.
 From what I remember, this was done (back in the days) so that clients 
could use something else than _svn if they wanted to. The env variable 
doesn't set the value but only has to be present.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: deprecate all of svn_wc.h ?

Posted by Stefan Küng <to...@gmail.com>.
On 09.04.2010 21:07, Greg Stein wrote:
> On Fri, Apr 9, 2010 at 13:49, Stefan Küng<to...@gmail.com>  wrote:
>> ...
>> Going through the code of TSVN, I found a few places where we use functions
>> defined in svn_wc.h:
>
> Ooh! Thanks!
>
>> * svn_wc_adm_probe_open3, used to get the svn_wc_adm_access_t required for
>> svn_client_uuid_from_path()
>
> Looks like we have an access-baton-free replacement
> (svn_client_uuid_from_path2).

Great! I was wondering why an svn_client_ function would need an access 
baton anyway.

>
>> * svn_wc_translated_file2
>> * svn_wc_prop_list
>> * svn_wc_is_adm_dir
>
> These should be fine.
>
>> * svn_wc_dup_status2
>
> I think we're going to be revamping the status structure pretty hard. It may

I'm aware that the status functions and structures will change a lot.

>> * svn_wc_get_pristine_copy_path (we don't use svn_wc_get_pristine_contents
>> because that returns a stream, and we need a file)
>
> Ugh. This one is particularly problematic.
>
> Can I ask what you use it for, so that we can come up with a suitable
> replacement?

We use it to get the pristine file when we do diffs WC->BASE. For 
example, users can doubleclick on modified files in most dialogs 
(especially the commit dialog) to show the diff before committing.

I know that there's svn_wc_get_pristine_contents which we should use 
instead, but we need a file, not a stream to work with (the file is 
needed because we start another app to do the diff). Sure, we could use 
the stream and save that ourselves to a file instead, but that's what 
svn_wc_get_pristine_copy_path already does, so we still use that api.

>
>> * svn_wc_set_adm_dir
>
> Set? Is this for when you detect a working copy that uses _svn ?

This is needed. Check the svn main.c file (for 1.6.x at least). There's 
this in there:

#if defined(WIN32) || defined(__CYGWIN__)
   /* Set the working copy administrative directory name. */
   if (getenv("SVN_ASP_DOT_NET_HACK"))
     {
       err = svn_wc_set_adm_dir("_svn", pool);
       if (err)
         return svn_cmdline_handle_exit_error(err, pool, "svn: ");
     }
#endif

So if the command line client has to initialize this, we have to do it too.
 From what I remember, this was done (back in the days) so that clients 
could use something else than _svn if they wanted to. The env variable 
doesn't set the value but only has to be present.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net

Re: deprecate all of svn_wc.h ?

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Apr 9, 2010 at 13:49, Stefan Küng <to...@gmail.com> wrote:
>...
> Going through the code of TSVN, I found a few places where we use functions
> defined in svn_wc.h:

Ooh! Thanks!

> * svn_wc_adm_probe_open3, used to get the svn_wc_adm_access_t required for
> svn_client_uuid_from_path()

Looks like we have an access-baton-free replacement
(svn_client_uuid_from_path2).

> * svn_wc_translated_file2
> * svn_wc_prop_list
> * svn_wc_is_adm_dir

These should be fine.

> * svn_wc_dup_status2

I think we're going to be revamping the status structure pretty hard. It may

> * svn_wc_get_pristine_copy_path (we don't use svn_wc_get_pristine_contents
> because that returns a stream, and we need a file)

Ugh. This one is particularly problematic.

Can I ask what you use it for, so that we can come up with a suitable
replacement?

> * svn_wc_set_adm_dir

Set? Is this for when you detect a working copy that uses _svn ?

> * svn_wc_match_ignore_list
>
>
> I hope I got all the svn_wc_ functions we use in TSVN listed here. A code
> search for this also returns hundreds of hits where we use the status struct
> and other defines/enums/structs which are returned and used by svn_client_
> functions, so I might have missed some functions in those hits.

Gotcha. I'll go grab a copy of the sources, and get a feel for those
struct bits. That will help to guide how we revamp the status system.

Thanks,
-g

Re: deprecate all of svn_wc.h ?

Posted by Stefan Küng <to...@gmail.com>.
On 08.04.2010 23:02, Greg Stein wrote:
> Hey all,
>
> It has come up a few times on IRC discussions: "we should never have
> exposed libsvn_wc, just libsvn_client".
>
> Well, we've already exposed it, so we need to at least keep that stuff
> around. But moving forward... should new functions continue to be
> exposed? Or should all new functions go into svn_wc_private.h?
>
> This question is (probably) directed at our (GUI) client developers.
> Do you ever use WC functions? And if you do, then which ones? Where is
> svn_client.h insufficient, leading you to use svn_wc.h APIs?
>
> Note: I think the separation is a Good Thing, for our benefit, but we
> don't necessarily have to expose the WC layer to downstream
> developers.

Going through the code of TSVN, I found a few places where we use 
functions defined in svn_wc.h:

* svn_wc_adm_probe_open3, used to get the svn_wc_adm_access_t required 
for svn_client_uuid_from_path()
* svn_wc_translated_file2
* svn_wc_prop_list
* svn_wc_is_adm_dir
* svn_wc_dup_status2
* svn_wc_get_pristine_copy_path (we don't use 
svn_wc_get_pristine_contents because that returns a stream, and we need 
a file)
* svn_wc_set_adm_dir
* svn_wc_match_ignore_list


I hope I got all the svn_wc_ functions we use in TSVN listed here. A 
code search for this also returns hundreds of hits where we use the 
status struct and other defines/enums/structs which are returned and 
used by svn_client_ functions, so I might have missed some functions in 
those hits.

Stefan


-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net