Julian Foad wrote on Fri, Jan 28, 2022 at 17:15:53 +0000:
> For interest/comments.

Please use text/* MIME type for patches so our MUAs show them inline.
Naming patches *.txt usually achieves this.

> On the 'pristines-on-demand' branch:
> 
>   - Sketch of configurable pristines-mode per WC.
>   - Debug/trace prints.
> 
> The pristines-mode config option can be set in:
> 
>   - user config (e.g. '~/.subversion/config') -- for user default
>   - (new) WC config file ('wc/.svn/config') -- per WC override
> 
> The pristines mode can:
> 
>   - skip the "hydrating" of modified files
>   - skip the "dehydrating" of unmodified files
> 
> It CANNOT yet:
> 
>   - force "hydrating" files that currently have no local modification
> 
> Syntax and currently effective values for the option (in the user
> config file and/or in the per-WC config file):
> 
>   [working-copy]
>   pristines-mode = hydrate=[never|on-demand],dehydrate=[never|on-demand]

The value "all", returned as the default value from svn_wc__db_pristines_mode(),
is not handled at all by check_pristines_mode().

Therefore, the resulting behaviour will be the default behaviour, namely
to fall back to the values of ALLOW_HYDRATE and ALLOW_DEHYDRATE
hardcoded into the svn_client__textbase_sync() callsite.

Should "all" mean ALLOW_HYDRATE and ALLOW_DEHYDRATE would both be forced
to TRUE?  Or perhaps should it be renamed "default"?

Or perhaps this patch is at too early a stage for any of this to matter :-)

Trimmed diff below.

> +++ subversion/libsvn_wc/textbase.c   (working copy)
> @@ -513,12 +516,52 @@ textbase_hydrate_cb(void *baton,
> +/* Check the pristines mode configured for the WC identified by WC_CTX,
> + * LOCAL_ABSPATH.  Modify the values of *ALLOW_HYDRATE, *ALLOW_DEHYDRATE
> + * according to the mode.

When the value is "all", *ALLOW_HYDRATE and *ALLOW_DEHYDRATE are not
modified.  That doesn't matter for the current callers, but a future
caller could assume that passing the address of an uninitialized
variable is allowed, in which case we'd go on using the uninitialized
value.

> + */
> +static svn_error_t *
> +check_pristines_mode(svn_boolean_t *allow_hydrate,
> +                     svn_boolean_t *allow_dehydrate,
> +                     svn_wc_context_t *wc_ctx,
> +                     const char *local_abspath,
> +                     apr_pool_t *scratch_pool)
> +{
> +  const char *mode;
> +
> +  SVN_ERR(svn_wc__db_pristines_mode(wc_ctx->db, local_abspath,
> +                                    &mode, scratch_pool));
> +
> +  /* ### for now, these are just a bunch of experimental modes and
> +   * experimental matching syntax */
> +  if (strcmp(mode, "off") == 0)
> +    {
> +      *allow_hydrate = FALSE;
> +      *allow_dehydrate = FALSE;
> +    }
> +  if (strcmp(mode, "off-line") == 0)
> +    *allow_hydrate = FALSE;
> +  if (strcmp(mode, "fetch-only") == 0)
> +    *allow_dehydrate = FALSE;
> +  if (strcmp(mode, "fetch-all") == 0)
> +    *allow_dehydrate = FALSE;
> +
> +  if (strstr(mode, "hydrate=never"))
> +    *allow_hydrate = FALSE;
> +  if (strstr(mode, "dehydrate=never"))
> +    *allow_dehydrate = FALSE;
> +
> +  SVN_DBG(("pristines mode: %s -> hydrate=%d dehydrate=%d",
> +           mode, *allow_hydrate, *allow_dehydrate));
> +  return SVN_NO_ERROR;
> +}
> +++ subversion/libsvn_wc/wc_db.h      (working copy)
> @@ -1069,12 +1069,21 @@ svn_wc__db_pristine_check(svn_boolean_t
> +/* Read the pristines mode into *MODE from the per-WC config (if set there)
> + * or else from the per-user config (if set there), with a default value of
> + * "all". */
> +svn_error_t *
> +svn_wc__db_pristines_mode(svn_wc__db_t *db,
> +                          const char *local_abspath,
> +                          const char **mode,
> +                          apr_pool_t *scratch_pool);
> +++ subversion/libsvn_wc/wc_db_wcroot.c       (working copy)
> @@ -229,21 +229,23 @@ svn_wc__db_open(svn_wc__db_t **db,
>  {
>    *db = apr_pcalloc(result_pool, sizeof(**db));
>    (*db)->config = config;
>    (*db)->verify_format = !open_without_upgrade;
>    (*db)->enforce_empty_wq = enforce_empty_wq;
>    (*db)->dir_data = apr_hash_make(result_pool);
> +  (*db)->pristines_mode = "all";

Cheers,

Daniel

Reply via email to