[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r19500 - in branches/changelist-feature/subversion: include libsvn_client libsvn_wc svn

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2006-05-05 21:03:55 CEST

sussman@tigris.org writes:

> Author: sussman
> Date: Tue May 2 14:17:57 2006
> New Revision: 19500
>
> Added:
> branches/changelist-feature/subversion/libsvn_client/changelist.c (contents, props changed)
> branches/changelist-feature/subversion/svn/changelist-cmd.c (contents, props changed)
> Modified:
> branches/changelist-feature/subversion/include/svn_client.h
> branches/changelist-feature/subversion/include/svn_wc.h
> branches/changelist-feature/subversion/libsvn_client/info.c
> branches/changelist-feature/subversion/libsvn_wc/adm_ops.c
> branches/changelist-feature/subversion/libsvn_wc/entries.c
> branches/changelist-feature/subversion/libsvn_wc/entries.h
> branches/changelist-feature/subversion/svn/cl.h
> branches/changelist-feature/subversion/svn/info-cmd.c
> branches/changelist-feature/subversion/svn/main.c
>
> Log:
> First bits of code for simple client-side changelist feature.
>
> Implement 'svn changelist' and make 'svn info' show the new field in
> .svn/entries.
>
>
> * subversion/include/svn_wc.h
>
> (svn_wc_entry_t): new 'changelist' field.
> (svn_wc_tweak_changelist): new declaration.
>
> * subversion/include/svn_client.h
>
> (svn_info_t): new 'changelist' field.
> (svn_client_changelist): new declaration.
>
> * subversion/libsvn_wc/entries.h
>
> (SVN_WC__ENTRY_ATTR_CHANGELIST, SVN_WC__ENTRY_MODIFY_CHANGELIST):
> new #defines.
>
> * subversion/libsvn_wc/entries.c
>
> (read_entry, svn_wc__atts_to_entry, write_entry, write_entry_xml,
> fold_entry, svn_wc_entry_dup): handle the new 'changelist' entry field.
>
> * subversion/libsvn_wc/adm_ops.c
>
> (svn_wc_tweak_changelist): new function.
>
> * subversion/libsvn_client/info.c
>
> (build_info_from_entry): handle new 'changelist' field.
>
> * subversion/libsvn_client/changelist.c: new file.
>
> * subversion/svn/cl.h
>
> (enum): add svn_cl__clear_opt.
> (svn_cl__opt_state_t): add 'clear' field.
> (svn_cl__changelist): new declaration.
>
> * subversion/svn/changelist-cmd.c: new file.
>
> * subversion/svn/info-cmd.c
>
> (print_info): display changelist field.
>
> * subversion/svn/main.c
>
> (svn_cl__options): add 'clear' option.
> (svn_cl__cmd_table): add 'changelist' command.
> (main): parse clear_opt.
>
>
>
> Modified: branches/changelist-feature/subversion/include/svn_client.h
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/include/svn_client.h?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/include/svn_client.h (original)
> +++ branches/changelist-feature/subversion/include/svn_client.h Tue May 2 14:17:57 2006
> @@ -2448,6 +2448,24 @@
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);
>
> +
> +/**
> + * Associate @a path with changelist @a changelist_name. If CLEAR is

@a clear

> + * set, then ignore @a changelist_name and deassociate any existing
> + * changelist from @a path.

Perhaps call it "changelist" rather than "changelist_name" so that it
matches the name in svn_info_t.

Perhaps mention the encoding, I guess it's UTF8?

Did you consider using two functions, set and clear?

> + *
> + * Note: this metadata is purely a client-side "bookkeeping"
> + * convenience, and is entirely managed by the working copy.
> + *
> + * @since New in 1.5.
> + */
> +svn_error_t *
> +svn_client_changelist(const char *path,
> + const char *changelist_name,
> + svn_boolean_t clear,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
>
> /** Locking commands
> *
> @@ -2582,6 +2600,7 @@
> const char *conflict_new;
> const char *conflict_wrk;
> const char *prejfile;
> + const char *changelist;

Needs an @since.

> /** @} */
>
> } svn_info_t;
>
> Modified: branches/changelist-feature/subversion/include/svn_wc.h
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/include/svn_wc.h?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/include/svn_wc.h (original)
> +++ branches/changelist-feature/subversion/include/svn_wc.h Tue May 2 14:17:57 2006
> @@ -1253,6 +1253,11 @@
> */
> apr_time_t lock_creation_date;
>
> + /** which changelist this item is part of, or NULL if not part of any.
> + * @since New in 1.5.
> + */
> + const char *changelist;
> +

If it's intended for 1.5 then this field has to come after all the 1.4
fields.

> /** Whether this entry has any working properties.
> * False if this information is not stored in the entry.
> *
> @@ -3448,6 +3453,24 @@
> void *cancel_baton,
> apr_pool_t *pool);
>
> +
> +/**
> + * Associate @a path with changelist @a changelist_name by setting the
> + * 'changelist' attribute in its entry. If CLEAR is set, then ignore

@a clear

> + * @a changelist_name and remove any 'changelist' attribute in @a
> + * path's entry.

"changelist" rather than "changelist_name"?

> + *
> + * Note: this metadata is purely a client-side "bookkeeping"
> + * convenience, and is entirely managed by the working copy.
> + *
> + * @since New in 1.5.
> + */
> +svn_error_t *
> +svn_wc_tweak_changelist(const char *path,
> + const char *changelist_name,
> + svn_boolean_t clear,
> + apr_pool_t *pool);

Any reason why this function name contains "tweak" while the client
version does not?

> +
>
> #ifdef __cplusplus
> }
>
> Added: branches/changelist-feature/subversion/libsvn_client/changelist.c
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/libsvn_client/changelist.c?pathrev=19500
> ==============================================================================
> --- (empty file)
> +++ branches/changelist-feature/subversion/libsvn_client/changelist.c Tue May 2 14:17:57 2006
> @@ -0,0 +1,51 @@
> +/*
> + * changelist.c: implementation of the 'changelist' command
> + *
> + * ====================================================================
> + * Copyright (c) 2006 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +/* ==================================================================== */
> +
> +
> +
> +/*** Includes. ***/
> +
> +#include "svn_client.h"
> +#include "svn_wc.h"
> +
> +
> +/*** Code. ***/
> +
> +svn_error_t *
> +svn_client_changelist(const char *path,
> + const char *changelist_name,
> + svn_boolean_t clear,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + SVN_ERR(svn_wc_tweak_changelist(path, changelist_name, clear, pool));
> +
> + /* ### TODO(sussman): create new notification type, and send
> + notification feedback. See locking-commands.c. */
> + if (! clear)
> + printf("Path '%s' is now part of changelist '%s'.\n",
> + path, changelist_name);
> + else
> + printf("Path '%s' is no longer associated with a changelist'.\n", path);
> +
> + return SVN_NO_ERROR;
> +}
> +
> +
>
> Modified: branches/changelist-feature/subversion/libsvn_client/info.c
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/libsvn_client/info.c?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/libsvn_client/info.c (original)
> +++ branches/changelist-feature/subversion/libsvn_client/info.c Tue May 2 14:17:57 2006
> @@ -89,6 +89,7 @@
> tmpinfo->conflict_new = entry->conflict_new;
> tmpinfo->conflict_wrk = entry->conflict_wrk;
> tmpinfo->prejfile = entry->prejfile;
> + tmpinfo->changelist = entry->changelist;
>
> /* lock stuff */
> if (entry->lock_token) /* the token is the critical bit. */
>
> Modified: branches/changelist-feature/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/libsvn_wc/adm_ops.c?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/libsvn_wc/adm_ops.c (original)
> +++ branches/changelist-feature/subversion/libsvn_wc/adm_ops.c Tue May 2 14:17:57 2006
> @@ -2388,3 +2388,35 @@
>
> return SVN_NO_ERROR;
> }
> +
> +
> +svn_error_t *
> +svn_wc_tweak_changelist(const char *path,
> + const char *changelist_name,
> + svn_boolean_t clear,
> + apr_pool_t *pool)
> +{
> + svn_wc_adm_access_t *adm_access;
> + const svn_wc_entry_t *entry;
> + svn_wc_entry_t newentry;
> +
> + SVN_ERR(svn_wc_adm_probe_open3(&adm_access, NULL, path,
> + TRUE, /* get write lock */
> + 0, /* depth */
> + NULL, NULL, pool));
> +
> + SVN_ERR(svn_wc_entry(&entry, path, adm_access, FALSE, pool));
> +
> + if (! entry)
> + return svn_error_createf(SVN_ERR_UNVERSIONED_RESOURCE, NULL,
> + _("'%s' is not under version control"), path);
> +
> + newentry.changelist = clear ? NULL : changelist_name;
> +
> + SVN_ERR(svn_wc__entry_modify(adm_access, entry->name, &newentry,
> + SVN_WC__ENTRY_MODIFY_CHANGELIST,
> + TRUE, pool));
> + SVN_ERR(svn_wc_adm_close(adm_access));
> +
> + return SVN_NO_ERROR;

I expect we will get requests to allow recursive operations, if so
then open/modify/write on each item approach might be a bottleneck.
It's probably best to make it recursive now (or document a strong
argument that will preempt the need to rev the interface :)

> +}
>
> Modified: branches/changelist-feature/subversion/libsvn_wc/entries.c
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/libsvn_wc/entries.c?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/libsvn_wc/entries.c (original)
> +++ branches/changelist-feature/subversion/libsvn_wc/entries.c Tue May 2 14:17:57 2006
> @@ -445,7 +445,10 @@
>
> /* Lock creation date. */
> SVN_ERR(read_time(&entry->lock_creation_date, buf, end, pool));
> -
> +
> + /* Changelist. */
> + SVN_ERR(read_str(&entry->changelist, buf, end, pool));
> +
> done:;
> *new_entry = entry;
> return SVN_NO_ERROR;
> @@ -784,6 +787,15 @@
> *modify_flags |= SVN_WC__ENTRY_MODIFY_LOCK_CREATION_DATE;
> }
> }
> +
> + /* changelist */
> + entry->changelist = apr_hash_get(atts, SVN_WC__ENTRY_ATTR_CHANGELIST,
> + APR_HASH_KEY_STRING);
> + if (entry->lock_owner)
> + {
> + *modify_flags |= SVN_WC__ENTRY_MODIFY_LOCK_OWNER;
> + entry->lock_owner = apr_pstrdup(pool, entry->lock_owner);
> + }

Why the lock_owner stuff, is it a trunk bug?

>
> /* has-props flag. */
> SVN_ERR(do_bool_attr(&entry->has_props,
> @@ -1527,7 +1539,10 @@
>
> /* Lock creation date. */
> write_time(buf, entry->lock_creation_date, pool);
> -
> +
> + /* Changelist. */
> + write_str(buf, entry->changelist, pool);
> +
> /* Remove redundant separators at the end of the entry. */
> while (buf->len > 1 && buf->data[buf->len - 2] == '\n')
> buf->len--;
> @@ -1712,6 +1727,11 @@
> APR_HASH_KEY_STRING,
> svn_time_to_cstring(entry->lock_creation_date, pool));
>
> + /* Changelist */
> + if (entry->changelist)
> + apr_hash_set(atts, SVN_WC__ENTRY_ATTR_CHANGELIST, APR_HASH_KEY_STRING,
> + entry->changelist);
> +
> /* Has-props flag. */
> apr_hash_set(atts, SVN_WC__ENTRY_ATTR_HAS_PROPS, APR_HASH_KEY_STRING,
> (entry->has_props ? "true" : NULL));
> @@ -2102,6 +2122,12 @@
> if (modify_flags & SVN_WC__ENTRY_MODIFY_LOCK_CREATION_DATE)
> cur_entry->lock_creation_date = entry->lock_creation_date;
>
> + /* Changelist */
> + if (modify_flags & SVN_WC__ENTRY_MODIFY_CHANGELIST)
> + cur_entry->changelist = (entry->changelist
> + ? apr_pstrdup(pool, entry->changelist)
> + : NULL);
> +
> /* has-props flag */
> if (modify_flags & SVN_WC__ENTRY_MODIFY_HAS_PROPS)
> cur_entry->has_props = entry->has_props;
> @@ -2511,6 +2537,10 @@
> dupentry->lock_owner = apr_pstrdup(pool, entry->lock_owner);
> if (entry->lock_comment)
> dupentry->lock_comment = apr_pstrdup(pool, entry->lock_comment);
> + if (entry->lock_comment)
> + dupentry->lock_comment = apr_pstrdup(pool, entry->lock_comment);

More lock_owner stuff.

> + if (entry->changelist)
> + dupentry->changelist = apr_pstrdup(pool, entry->changelist);
> if (entry->cachable_props)
> dupentry->cachable_props = apr_pstrdup(pool, entry->cachable_props);
> if (entry->present_props)
>
> Modified: branches/changelist-feature/subversion/libsvn_wc/entries.h
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/libsvn_wc/entries.h?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/libsvn_wc/entries.h (original)
> +++ branches/changelist-feature/subversion/libsvn_wc/entries.h Tue May 2 14:17:57 2006
> @@ -71,6 +71,7 @@
> #define SVN_WC__ENTRY_ATTR_HAS_PROP_MODS "has-prop-mods"
> #define SVN_WC__ENTRY_ATTR_CACHABLE_PROPS "cachable-props"
> #define SVN_WC__ENTRY_ATTR_PRESENT_PROPS "present-props"
> +#define SVN_WC__ENTRY_ATTR_CHANGELIST "changelist"
>
> /* Attribute values for 'schedule' */
> #define SVN_WC__ENTRY_VALUE_ADD "add"
> @@ -146,6 +147,7 @@
> #define SVN_WC__ENTRY_MODIFY_HAS_PROP_MODS 0x08000000
> #define SVN_WC__ENTRY_MODIFY_CACHABLE_PROPS 0x10000000
> #define SVN_WC__ENTRY_MODIFY_PRESENT_PROPS 0x20000000
> +#define SVN_WC__ENTRY_MODIFY_CHANGELIST 0x40000000
>
>
> /* ...or perhaps this to mean all of those above... */
>
> Added: branches/changelist-feature/subversion/svn/changelist-cmd.c
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/svn/changelist-cmd.c?pathrev=19500
> ==============================================================================
> --- (empty file)
> +++ branches/changelist-feature/subversion/svn/changelist-cmd.c Tue May 2 14:17:57 2006
> @@ -0,0 +1,72 @@
> +/*
> + * changelist-cmd.c -- Associate (or deassociate) a wc path with a changelist.
> + *
> + * ====================================================================
> + * Copyright (c) 2006 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +/* ==================================================================== */
> +
> +
> +
> +/*** Includes. ***/
> +
> +#include "svn_pools.h"
> +#include "svn_client.h"
> +#include "svn_error.h"
> +#include "cl.h"
> +
> +
> +/*** Code. ***/
> +
> +
> +/* This implements the `svn_opt_subcommand_t' interface. */
> +svn_error_t *
> +svn_cl__changelist(apr_getopt_t *os,
> + void *baton,
> + apr_pool_t *pool)
> +{
> + int i;
> + const char *changelist_name;
> + svn_cl__opt_state_t *opt_state = ((svn_cl__cmd_baton_t *) baton)->opt_state;
> + svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
> + apr_array_header_t *targets;
> + apr_pool_t *subpool = svn_pool_create(pool);
> +
> + SVN_ERR(svn_opt_args_to_target_array2(&targets, os,
> + opt_state->targets, pool));
> +
> + if (targets->nelts < 2)
> + return svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, 0, NULL);

What about an implicit "."?

> +
> + changelist_name = ((const char **) (targets->elts))[0];
> +
> + for (i = 1; i < targets->nelts; i++)
> + {
> + const char *target = ((const char **) (targets->elts))[i];
> +
> + svn_pool_clear(subpool);
> + SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
> + SVN_ERR(svn_cl__try
> + (svn_client_changelist(target, changelist_name,
> + opt_state->clear, ctx, subpool),
> + NULL, opt_state->quiet,
> + SVN_ERR_UNVERSIONED_RESOURCE,
> + SVN_ERR_WC_PATH_NOT_FOUND,
> + SVN_NO_ERROR));
> + }
> + svn_pool_destroy(subpool);
> +
> + return SVN_NO_ERROR;
> +}
>
> Modified: branches/changelist-feature/subversion/svn/cl.h
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/svn/cl.h?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/svn/cl.h (original)
> +++ branches/changelist-feature/subversion/svn/cl.h Tue May 2 14:17:57 2006
> @@ -49,6 +49,7 @@
> svn_cl__auth_username_opt,
> svn_cl__autoprops_opt,
> svn_cl__config_dir_opt,
> + svn_cl__clear_opt,
> svn_cl__diff_cmd_opt,
> svn_cl__dry_run_opt,
> svn_cl__editor_cmd_opt,
> @@ -143,6 +144,7 @@
> svn_boolean_t no_autoprops; /* disable automatic properties */
> const char *native_eol; /* override system standard eol marker */
> svn_boolean_t summarize; /* create a summary of a diff */
> + svn_boolean_t clear; /* deassociate a changelist */
> } svn_cl__opt_state_t;
>
>
> @@ -157,6 +159,7 @@
> svn_opt_subcommand_t
> svn_cl__add,
> svn_cl__blame,
> + svn_cl__changelist,
> svn_cl__checkout,
> svn_cl__cleanup,
> svn_cl__commit,
>
> Modified: branches/changelist-feature/subversion/svn/info-cmd.c
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/svn/info-cmd.c?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/svn/info-cmd.c (original)
> +++ branches/changelist-feature/subversion/svn/info-cmd.c Tue May 2 14:17:57 2006
> @@ -417,6 +417,10 @@
> }
> }
>
> + if (info->changelist)
> + SVN_ERR(svn_cmdline_printf(pool, _("Changelist: %s\n"),
> + info->changelist));
> +
> /* Print extra newline separator. */
> SVN_ERR(svn_cmdline_printf(pool, "\n"));
>
>
> Modified: branches/changelist-feature/subversion/svn/main.c
> URL: http://svn.collab.net/viewvc/svn/branches/changelist-feature/subversion/svn/main.c?pathrev=19500&r1=19499&r2=19500
> ==============================================================================
> --- branches/changelist-feature/subversion/svn/main.c (original)
> +++ branches/changelist-feature/subversion/svn/main.c Tue May 2 14:17:57 2006
> @@ -157,6 +157,8 @@
> N_("don't unlock the targets")},
> {"summarize", svn_cl__summarize, 0,
> N_("show a summary of the results")},
> + {"clear", svn_cl__clear_opt, 0,
> + N_("remove changelist association")},
> {0, 0, 0, 0}
> };
>
> @@ -220,6 +222,11 @@
> " looked up.\n"),
> {'r', SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },
>
> + { "changelist", svn_cl__changelist, {"cl"}, N_
> + ("Associate working copy paths with a changelist CLNAME.\n"
> + "usage: changelist CLNAME TARGET...\n"),
> + { svn_cl__clear_opt } },

How about svn_cl__targets_opt?

> +
> { "checkout", svn_cl__checkout, {"co"}, N_
> ("Check out a working copy from a repository.\n"
> "usage: checkout URL[@REV]... [PATH]\n"
> @@ -1165,6 +1172,8 @@
> case svn_cl__summarize:
> opt_state.summarize = TRUE;
> break;
> + case svn_cl__clear_opt:
> + opt_state.clear = TRUE;
> default:
> /* Hmmm. Perhaps this would be a good place to squirrel away
> opts that commands like svn diff might need. Hmmm indeed. */
>

It's a bit of an odd interface, given:

$ svn changelist change1 foo bar
$ svn changelist change2 zig zag

I have to give a changelist name here:

$ svn changelist --clear change2 foo zag

but it's more or less redundant. Should that command clear foo from
the change1 changelist or should it warn that foo is not part of
change2?

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 5 21:04:35 2006

This is an archived mail posted to the Subversion Dev mailing list.