You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2011/05/17 09:51:00 UTC

Re: svn commit: r1103771 - in /subversion/trunk/subversion: libsvn_client/prop_commands.c svn/propedit-cmd.c svn/propget-cmd.c

hwright@apache.org wrote on Mon, May 16, 2011 at 15:36:24 -0000:
> Author: hwright
> Date: Mon May 16 15:36:24 2011
> New Revision: 1103771
> 
> URL: http://svn.apache.org/viewvc?rev=1103771&view=rev
> Log:
> Convert a bit of the recursive propget to use absolute paths, by doing path
> relativifcation in the command line program, where it belongs.
> 
> Note: I *think* this is backwards compatible, since we promise paths, but
> don't specify absolute or relative in the docs.  If somebody has an alternative
> interpretation, we can do the API-rev dance.
> 
> * subversion/svn/propget-cmd.c
>   (print_properties): Make a relative path from an absolute one, if possible.
> 
> * subversion/svn/propedit-cmd.c
>   (svn_cl__propedit): Fetch the property from the hash via absolute path.

How can it be a backwards compatible API change if you had to change the
API consumer (the cmdline client)?

Re: svn commit: r1103771 - in /subversion/trunk/subversion: libsvn_client/prop_commands.c svn/propedit-cmd.c svn/propget-cmd.c

Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, May 17, 2011 at 11:49 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Hyrum K Wright wrote on Tue, May 17, 2011 at 07:21:43 +0000:
>> On Tue, May 17, 2011 at 7:51 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>> > hwright@apache.org wrote on Mon, May 16, 2011 at 15:36:24 -0000:
>> >> Author: hwright
>> >> Date: Mon May 16 15:36:24 2011
>> >> New Revision: 1103771
>> >>
>> >> URL: http://svn.apache.org/viewvc?rev=1103771&view=rev
>> >> Log:
>> >> Convert a bit of the recursive propget to use absolute paths, by doing path
>> >> relativifcation in the command line program, where it belongs.
>> >>
>> >> Note: I *think* this is backwards compatible, since we promise paths, but
>> >> don't specify absolute or relative in the docs.  If somebody has an alternative
>> >> interpretation, we can do the API-rev dance.
>> >>
>> >> * subversion/svn/propget-cmd.c
>> >>   (print_properties): Make a relative path from an absolute one, if possible.
>> >>
>> >> * subversion/svn/propedit-cmd.c
>> >>   (svn_cl__propedit): Fetch the property from the hash via absolute path.
>> >
>> > How can it be a backwards compatible API change if you had to change the
>> > API consumer (the cmdline client)?
>>
>> If the consumer depends upon an (undocumented) implementation detail,
>> you can bet it'll need to change when those details change.  (Example:
>> anybody relying upon the entries-file implementation detail is going
>> to have a hard time with 1.7.)
>>
>> I maintain that if we didn't document the behavior, existing clients
>> are depending on an implementation detail (which they shouldn't be
>> doing).  This doesn't mean we can't rev the API to make things easier
>> for them, but it does mean we are well within our rights to change the
>> implementation and have things break for the consumer.
>>
>
> Two things:
>
> * Could we have written the cmdline client in 1.6 in a way that's
>  oblivious of the absoluteness/relativeness of the hash keys?
>
> * Concretely: the svn_client_propget3() docstring states that
>  the keys will be prefixed by @a TARGET.  That seems to me to
>  imply svn_dirent_join(TARGET, ...) --- without changing whether
>  TARGET is absolute or relative.

Yes. I don't have a strong opinion in this discussion, but completely
objectively, one could also say that the documentation was flawed (or
inaccurate), i.e. a bug in the docstring. It talked about a PATH, but
every consumer deduced it had to be a relative path ...

-- 
Johan

Re: svn commit: r1103771 - in /subversion/trunk/subversion: libsvn_client/prop_commands.c svn/propedit-cmd.c svn/propget-cmd.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K Wright wrote on Tue, May 17, 2011 at 07:21:43 +0000:
> On Tue, May 17, 2011 at 7:51 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > hwright@apache.org wrote on Mon, May 16, 2011 at 15:36:24 -0000:
> >> Author: hwright
> >> Date: Mon May 16 15:36:24 2011
> >> New Revision: 1103771
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1103771&view=rev
> >> Log:
> >> Convert a bit of the recursive propget to use absolute paths, by doing path
> >> relativifcation in the command line program, where it belongs.
> >>
> >> Note: I *think* this is backwards compatible, since we promise paths, but
> >> don't specify absolute or relative in the docs.  If somebody has an alternative
> >> interpretation, we can do the API-rev dance.
> >>
> >> * subversion/svn/propget-cmd.c
> >>   (print_properties): Make a relative path from an absolute one, if possible.
> >>
> >> * subversion/svn/propedit-cmd.c
> >>   (svn_cl__propedit): Fetch the property from the hash via absolute path.
> >
> > How can it be a backwards compatible API change if you had to change the
> > API consumer (the cmdline client)?
> 
> If the consumer depends upon an (undocumented) implementation detail,
> you can bet it'll need to change when those details change.  (Example:
> anybody relying upon the entries-file implementation detail is going
> to have a hard time with 1.7.)
> 
> I maintain that if we didn't document the behavior, existing clients
> are depending on an implementation detail (which they shouldn't be
> doing).  This doesn't mean we can't rev the API to make things easier
> for them, but it does mean we are well within our rights to change the
> implementation and have things break for the consumer.
> 

Two things:

* Could we have written the cmdline client in 1.6 in a way that's
  oblivious of the absoluteness/relativeness of the hash keys?

* Concretely: the svn_client_propget3() docstring states that
  the keys will be prefixed by @a TARGET.  That seems to me to
  imply svn_dirent_join(TARGET, ...) --- without changing whether
  TARGET is absolute or relative.

> -Hyrum

Re: svn commit: r1103771 - in /subversion/trunk/subversion: libsvn_client/prop_commands.c svn/propedit-cmd.c svn/propget-cmd.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K Wright wrote on Tue, May 17, 2011 at 07:21:43 +0000:
> On Tue, May 17, 2011 at 7:51 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > hwright@apache.org wrote on Mon, May 16, 2011 at 15:36:24 -0000:
> >> Author: hwright
> >> Date: Mon May 16 15:36:24 2011
> >> New Revision: 1103771
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1103771&view=rev
> >> Log:
> >> Convert a bit of the recursive propget to use absolute paths, by doing path
> >> relativifcation in the command line program, where it belongs.
> >>
> >> Note: I *think* this is backwards compatible, since we promise paths, but
> >> don't specify absolute or relative in the docs.  If somebody has an alternative
> >> interpretation, we can do the API-rev dance.
> >>
> >> * subversion/svn/propget-cmd.c
> >>   (print_properties): Make a relative path from an absolute one, if possible.
> >>
> >> * subversion/svn/propedit-cmd.c
> >>   (svn_cl__propedit): Fetch the property from the hash via absolute path.
> >
> > How can it be a backwards compatible API change if you had to change the
> > API consumer (the cmdline client)?
> 
> If the consumer depends upon an (undocumented) implementation detail,
> you can bet it'll need to change when those details change.  (Example:
> anybody relying upon the entries-file implementation detail is going
> to have a hard time with 1.7.)
> 
> I maintain that if we didn't document the behavior, existing clients
> are depending on an implementation detail (which they shouldn't be
> doing).  This doesn't mean we can't rev the API to make things easier
> for them, but it does mean we are well within our rights to change the
> implementation and have things break for the consumer.
> 

Two things:

* Could we have written the cmdline client in 1.6 in a way that's
  oblivious of the absoluteness/relativeness of the hash keys?

* Concretely: the svn_client_propget3() docstring states that
  the keys will be prefixed by @a TARGET.  That seems to me to
  imply svn_dirent_join(TARGET, ...) --- without changing whether
  TARGET is absolute or relative.

> -Hyrum

Re: svn commit: r1103771 - in /subversion/trunk/subversion: libsvn_client/prop_commands.c svn/propedit-cmd.c svn/propget-cmd.c

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Tue, May 17, 2011 at 7:51 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> hwright@apache.org wrote on Mon, May 16, 2011 at 15:36:24 -0000:
>> Author: hwright
>> Date: Mon May 16 15:36:24 2011
>> New Revision: 1103771
>>
>> URL: http://svn.apache.org/viewvc?rev=1103771&view=rev
>> Log:
>> Convert a bit of the recursive propget to use absolute paths, by doing path
>> relativifcation in the command line program, where it belongs.
>>
>> Note: I *think* this is backwards compatible, since we promise paths, but
>> don't specify absolute or relative in the docs.  If somebody has an alternative
>> interpretation, we can do the API-rev dance.
>>
>> * subversion/svn/propget-cmd.c
>>   (print_properties): Make a relative path from an absolute one, if possible.
>>
>> * subversion/svn/propedit-cmd.c
>>   (svn_cl__propedit): Fetch the property from the hash via absolute path.
>
> How can it be a backwards compatible API change if you had to change the
> API consumer (the cmdline client)?

If the consumer depends upon an (undocumented) implementation detail,
you can bet it'll need to change when those details change.  (Example:
anybody relying upon the entries-file implementation detail is going
to have a hard time with 1.7.)

I maintain that if we didn't document the behavior, existing clients
are depending on an implementation detail (which they shouldn't be
doing).  This doesn't mean we can't rev the API to make things easier
for them, but it does mean we are well within our rights to change the
implementation and have things break for the consumer.

-Hyrum

Re: svn commit: r1103771 - in /subversion/trunk/subversion: libsvn_client/prop_commands.c svn/propedit-cmd.c svn/propget-cmd.c

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Tue, May 17, 2011 at 7:51 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> hwright@apache.org wrote on Mon, May 16, 2011 at 15:36:24 -0000:
>> Author: hwright
>> Date: Mon May 16 15:36:24 2011
>> New Revision: 1103771
>>
>> URL: http://svn.apache.org/viewvc?rev=1103771&view=rev
>> Log:
>> Convert a bit of the recursive propget to use absolute paths, by doing path
>> relativifcation in the command line program, where it belongs.
>>
>> Note: I *think* this is backwards compatible, since we promise paths, but
>> don't specify absolute or relative in the docs.  If somebody has an alternative
>> interpretation, we can do the API-rev dance.
>>
>> * subversion/svn/propget-cmd.c
>>   (print_properties): Make a relative path from an absolute one, if possible.
>>
>> * subversion/svn/propedit-cmd.c
>>   (svn_cl__propedit): Fetch the property from the hash via absolute path.
>
> How can it be a backwards compatible API change if you had to change the
> API consumer (the cmdline client)?

If the consumer depends upon an (undocumented) implementation detail,
you can bet it'll need to change when those details change.  (Example:
anybody relying upon the entries-file implementation detail is going
to have a hard time with 1.7.)

I maintain that if we didn't document the behavior, existing clients
are depending on an implementation detail (which they shouldn't be
doing).  This doesn't mean we can't rev the API to make things easier
for them, but it does mean we are well within our rights to change the
implementation and have things break for the consumer.

-Hyrum