Re: svn commit: r1905663 - in /subversion/branches/pristines-on-demand-on-mwf/subversion: include/ include/private/ libsvn_client/ libsvn_wc/

[Evgeny Kotkov via dev]

Bert Huijben  writes:

> All the now deprecated functions now fail unconditionally when the setting
> is enabled. Isn’t it possible to do this more graceful whenever a file is
> encountered which misses it’s prisite version?

The problem with this approach is that the functions are going to work, but
only *sometimes*, and will fail unpredictably, depending on whether a pristine
was fetched or removed by any previous API call.

With that in mind, failing consistently seems to be a more appropriate choice
for a deprecated function than failing randomly.

> As far as I know it is expected that some of the files do have pristines,
> while others don’t… That would allow things like diffs on old clients that
> didn’t switch apis yet.

Thinking about this, the currently documented assumption is that a file has
no pristine if and only if it's being added/replaced without a copyfrom.
So maybe we cannot really extend that to "any file might not have a pristine"
without it being an incompatible change.


Thanks,
Evgeny Kotkov

RE: svn commit: r1905663 - in /subversion/branches/pristines-on-demand-on-mwf/subversion: include/ include/private/ libsvn_client/ libsvn_wc/

[Bert Huijben]

All the now deprecated functions now fail unconditionally when the setting is 
enabled. Isn’t it possible to do this more graceful whenever a file is 
encountered which misses it’s prisite version?

 �

As far as I know it is expected that some of the files do have pristines, while 
others don’t… That would allow things like diffs on old clients that didn’t 
switch apis yet.

 �

 �

And in many cases these clients might just pass a wc-ctx that was created with 
the client api, so they might even have support for obtaining the pristines via 
callbacks without even knowing… but now they fail because we just check a 
single boolean in the deprecated api.

 �

 �

I’m not even sure if this is really worth revving the apis without altering the 
arguments… We introduced new error codes on existing apis before, like when we 
switched to WC-NG.

 �

Bert

 �

From: Daniel Sahlberg  
Sent: Thursday, December 1, 2022 1:14 PM
To: dev@subversion.apache.org
Cc: comm...@subversion.apache.org
Subject: Re: svn commit: r1905663 - in 
/subversion/branches/pristines-on-demand-on-mwf/subversion: include/ 
include/private/ libsvn_client/ libsvn_wc/

 �

Den tors 1 dec. 2022 kl 11:42 skrev mailto:kot...@apache.org> >:

Author: kotkov
Date: Thu Dec � 1 10:42:41 2022
New Revision: 1905663

 �

[...] �

 �

Modified: 
subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
URL: 
http://svn.apache.org/viewvc/subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h?rev=1905663
 

 =1905662=1905663=diff
==
--- 
subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
 (original)
+++ 
subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
 Thu Dec � 1 10:42:41 2022
@@ -581,6 +581,12 @@ SVN_ERROR_START
 �  �  �  �  �  �  � SVN_ERR_WC_CATEGORY_START + 42,
 �  �  �  �  �  �  � "Incompatible working copy settings")

+ � /** @since New in 1.15 */
+ � SVN_ERRDEF(SVN_ERR_WC_DEPRECATED_API_STORE_PRISTINE,
+ �  �  �  �  �  �  �SVN_ERR_WC_CATEGORY_START + 43,
+ �  �  �  �  �  �  �"This client was not updated to support working copies "
+ �  �  �  �  �  �  �"without local pristines")
+
 �  �/* fs errors */

 �

Is it really "This client"? It looks more to be based on the WC setting.

 �

Kind regards,

/Daniel

 �

Re: svn commit: r1905663 - in /subversion/branches/pristines-on-demand-on-mwf/subversion: include/ include/private/ libsvn_client/ libsvn_wc/

[Evgeny Kotkov via dev]

Daniel Sahlberg  writes:

>> +  /** @since New in 1.15 */
>> +  SVN_ERRDEF(SVN_ERR_WC_DEPRECATED_API_STORE_PRISTINE,
>> + SVN_ERR_WC_CATEGORY_START + 43,
>> + "This client was not updated to support working copies "
>> + "without local pristines")
>> +
>>/* fs errors */
>
> Is it really "This client"? It looks more to be based on the WC setting.

This new error should only occur when someone (a 3rd party client) uses the
deprecated API function for a working copy without pristines.

>From that point of view, I would say that the problem is in the client that is
still using the deprecated API and thus cannot properly handle such working
copies.

Also, the current message aims to be somewhat consistent with another existing
error for a case when a client is too old to read a newer working copy format:
 "This client is too old to work with the working copy […] (format 32)"

(After writing this, I realized that the "was not updated" part of the message
 may be misinterpreted as a requirement to update to the newer version of the
 software.  In r1905682, I slightly rephrased that part so that it would say:
   "This client uses a deprecated API that does not support working copies
without local pristines")


Thanks,
Evgeny Kotkov

Re: svn commit: r1905663 - in /subversion/branches/pristines-on-demand-on-mwf/subversion: include/ include/private/ libsvn_client/ libsvn_wc/

[Daniel Sahlberg]

Den tors 1 dec. 2022 kl 11:42 skrev :

> Author: kotkov
> Date: Thu Dec  1 10:42:41 2022
> New Revision: 1905663
>

[...]


> Modified:
> subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
> URL:
> http://svn.apache.org/viewvc/subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h?rev=1905663=1905662=1905663=diff
>
> ==
> ---
> subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
> (original)
> +++
> subversion/branches/pristines-on-demand-on-mwf/subversion/include/svn_error_codes.h
> Thu Dec  1 10:42:41 2022
> @@ -581,6 +581,12 @@ SVN_ERROR_START
>   SVN_ERR_WC_CATEGORY_START + 42,
>   "Incompatible working copy settings")
>
> +  /** @since New in 1.15 */
> +  SVN_ERRDEF(SVN_ERR_WC_DEPRECATED_API_STORE_PRISTINE,
> + SVN_ERR_WC_CATEGORY_START + 43,
> + "This client was not updated to support working copies "
> + "without local pristines")
> +
>/* fs errors */
>

Is it really "This client"? It looks more to be based on the WC setting.

Kind regards,
/Daniel