On Fri, Jun 6, 2008 at 8:20 AM, Blair Zajac <blair_at_orcaware.com> wrote:
> I'm working on getting intra-repository file externals into svn's wc, as an
> addition to our support for directory externals.
>
> A couple of design questions, but first a design point. I'm implementing
> file externals using the existing support in 1.5 that you can do 'touch foo;
> svn add foo; svn switch foo URL'. So under the hood, having a file external
> sets up a switched file. The existence of some additional fields in the
> entries will tell the client that it still needs to be treated specially.
I haven't completely thought through exactly what you're saying, but
anything that makes "externals" more like "repository-defined
suggestions for switches" is great in my book.
> Because the svn:externals property can add an external to subdirectories and
> users may do updates from a subdirectory, svn may not know that a switched
> path is a file external, so I need to add a marker in the wc entries that
> something is a file external and not just a switched path.
>
> So I can add a single entry line into .svn/entries with a single line,
> either of the two following form:
>
> [HEAD|r\d+] URL
>
> or two separate lines:
>
> URL
> [HEAD|r\d+]
>
> The two have to both be there for this to work, so maybe the first?
Parsing sucks. Later we'll decide that you can support, say, the
{DATE} revision type, which can have a space in it. Be less
ambiguous.
(and re: debuggability, as Karl mentioned: I disbelieve that anybody
is smart enough to consistently figure out what line is what in the
entries file by hand. But do see my svn-entries.el.)
--dave
> Also, representing a HEAD or fixed revision number the svn_opt_revision_t
> seems natural for this, but this has a number of different revision types
> that I don't need. Should I have a new smaller union type that just
> reflects a HEAD revision and a fixed revision?
>
> Any style feedback welcome.
>
> Patch for two separate lines in .svn/entries is below.
>
> Oh yes, and why do we have svn_wc__atts_to_entry()? This looks like it's
> more XML entries files related?
>
> Blair
>
>
>
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 31613)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -1861,6 +1861,23 @@
> * @since New in 1.5. */
> svn_depth_t depth;
>
> + /** The entry is a file external and this is the repository root
> + * relative path to the file, otherwise NULL if the file is not a
> + * file external.
> + *
> + * @since New in 1.6. */
> + const char *file_external_path;
> +
> + /** The entry is a file external and this is the revision number
> + * that the external is checked out from. The only permissable
> + * values are svn_opt_revision_unspecified if the entry is not an
> + * external, svn_opt_revision_head if the external revision is
> + * unspecified or specified with -r HEAD or svn_opt_revision_number
> + * for a specifiec revision number.
> + *
> + * @since New in 1.6. */
> + svn_opt_revision_t file_external_rev;
> +
> /* IMPORTANT: If you extend this structure, check the following functions
> in
> * subversion/libsvn_wc/entries.c, to see if you need to extend them as
> well.
> *
> Index: subversion/libsvn_wc/entries.c
> ===================================================================
> --- subversion/libsvn_wc/entries.c (revision 31613)
> +++ subversion/libsvn_wc/entries.c (working copy)
> @@ -2,7 +2,7 @@
> * entries.c : manipulating the administrative `entries' file.
> *
> * ====================================================================
> - * Copyright (c) 2000-2007 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2008 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
> @@ -292,6 +292,41 @@
> return SVN_NO_ERROR;
> }
>
> +/* Parse a file external revision number from a NULL terminated string
> + REV_STR into *RESULT. Valid strings are NULL, "HEAD", or r\d+. A
> + NULL string converts to a HEAD revision. */
> +static svn_error_t *
> +parse_file_external_rev(svn_opt_revision_t *result, const char *rev_str,
> + const char *name)
> +{
> + svn_opt_revision_t r;
> +
> + if (! rev_str)
> + r.kind = svn_opt_revision_head;
> + else
> + {
> + if (strcmp(rev_str, SVN_WC__ENTRY_VALUE_HEAD) == 0)
> + {
> + }
> + else if (rev_str[0] == 'r')
> + {
> + svn_revnum_t rev;
> + SVN_ERR(svn_revnum_parse(&rev, rev_str+1, NULL));
> + r.kind = svn_opt_revision_number;
> + r.value.number = rev;
> + }
> + else
> + return svn_error_createf(SVN_ERR_WC_CORRUPT, NULL,
> + _("Entry for '%s' has invalid file external
> "
> + "revision '%s'"),
> + name ? name : SVN_WC_ENTRY_THIS_DIR,
> + rev_str);
> + }
> + *result = r;
> +
> + return SVN_NO_ERROR;
> +}
> +
> /* Allocate an entry from POOL and read it from [*BUF, END). The
> buffer may be modified in place while parsing. Return the new
> entry in *NEW_ENTRY. Advance *BUF to point at the end of the entry
> @@ -498,6 +533,22 @@
> }
> MAYBE_DONE;
>
> + /* File external relative path. */
> + SVN_ERR(read_str(&entry->file_external_path, buf, end, pool));
> + MAYBE_DONE;
> +
> + /* File external revision. */
> + {
> + const char *rev_str;
> +
> + entry->file_external_rev.kind = svn_opt_revision_head;
> +
> + SVN_ERR(read_str(&rev_str, buf, end, pool));
> + SVN_ERR(parse_file_external_rev(&(entry->file_external_rev), rev_str,
> + name));
> + }
> + MAYBE_DONE;
> +
> done:
> *new_entry = entry;
> return SVN_NO_ERROR;
> @@ -911,6 +962,30 @@
> }
> }
>
> + /* File externals */
> + entry->file_external_path =
> + apr_hash_get(atts, SVN_WC__ENTRY_ATTR_FILE_EXTERNAL_PATH,
> + APR_HASH_KEY_STRING);
> + if (entry->file_external_path)
> + {
> + *modify_flags |= SVN_WC__ENTRY_MODIFY_FILE_EXTERNAL_PATH;
> + entry->file_external_path = apr_pstrdup(pool,
> entry->file_external_path);
> + }
> +
> + {
> + const char *rev_str;
> +
> + rev_str = apr_hash_get(atts, SVN_WC__ENTRY_ATTR_FILE_EXTERNAL_REV,
> + APR_HASH_KEY_STRING);
> + if (rev_str)
> + {
> + SVN_ERR(parse_file_external_rev(&(entry->file_external_rev),
> + rev_str,
> + NULL));
> + *modify_flags |= SVN_WC__ENTRY_MODIFY_FILE_EXTERNAL_REV;
> + }
> + }
> +
> *new_entry = entry;
> return SVN_NO_ERROR;
> }
> @@ -1664,6 +1739,27 @@
> write_val(buf, val, strlen(val));
> }
>
> + /* File externals. */
> + write_str(buf, entry->file_external_path, pool);
> + switch (entry->file_external_rev.kind)
> + {
> + case svn_opt_revision_head:
> + write_val(buf, SVN_WC__ENTRY_VALUE_HEAD,
> + sizeof(SVN_WC__ENTRY_VALUE_HEAD) - 1);
> + break;
> + case svn_opt_revision_number:
> + if (SVN_IS_VALID_REVNUM(entry->file_external_rev.value.number))
> + {
> + svn_stringbuf_appendbytes(buf, "r", 1);
> + write_revnum(buf, entry->file_external_rev.value.number, pool);
> + }
> + else
> + write_val(buf, NULL, 0);
> + break;
> + default:
> + write_val(buf, NULL, 0);
> + }
> +
> /* Remove redundant separators at the end of the entry. */
> while (buf->len > 1 && buf->data[buf->len - 2] == '\n')
> buf->len--;
> @@ -2311,6 +2407,16 @@
> cur_entry->keep_local = FALSE;
> }
>
> + /* File externals. */
> + if (modify_flags & SVN_WC__ENTRY_MODIFY_FILE_EXTERNAL_PATH)
> + cur_entry->file_external_path = (entry->file_external_path
> + ? apr_pstrdup(pool,
> +
> entry->file_external_path)
> + : NULL);
> +
> + if (modify_flags & SVN_WC__ENTRY_MODIFY_FILE_EXTERNAL_REV)
> + cur_entry->file_external_rev = entry->file_external_rev;
> +
> /* Make sure the entry exists in the entries hash. Possibly it
> already did, in which case this could have been skipped, but what
> the heck. */
> @@ -2675,6 +2781,9 @@
> dupentry->cachable_props = apr_pstrdup(pool, entry->cachable_props);
> if (entry->present_props)
> dupentry->present_props = apr_pstrdup(pool, entry->present_props);
> + if (entry->file_external_path)
> + dupentry->file_external_path = apr_pstrdup(pool,
> + entry->file_external_path);
> return dupentry;
> }
>
> Index: subversion/libsvn_wc/wc.h
> ===================================================================
> --- subversion/libsvn_wc/wc.h (revision 31613)
> +++ subversion/libsvn_wc/wc.h (working copy)
> @@ -2,7 +2,7 @@
> * wc.h : shared stuff internal to the svn_wc library.
> *
> * ====================================================================
> - * Copyright (c) 2000-2007 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2008 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
> @@ -69,9 +69,11 @@
> * The change from 8 to 9 was the addition of changelists, keep-local,
> * and sticky depth (for selective/sparse checkouts).
> *
> + * The change from 9 to 10 was the addition of file externals.
> + *
> * Please document any further format changes here.
> */
> -#define SVN_WC__VERSION 9
> +#define SVN_WC__VERSION 10
>
> /* A version <= this doesn't have property caching in the entries file. */
> #define SVN_WC__NO_PROPCACHING_VERSION 5
> Index: subversion/libsvn_wc/entries.h
> ===================================================================
> --- subversion/libsvn_wc/entries.h (revision 31613)
> +++ subversion/libsvn_wc/entries.h (working copy)
> @@ -77,12 +77,16 @@
> #define SVN_WC__ENTRY_ATTR_CHANGELIST "changelist"
> #define SVN_WC__ENTRY_ATTR_KEEP_LOCAL "keep-local"
> #define SVN_WC__ENTRY_ATTR_WORKING_SIZE "working-size"
> +#define SVN_WC__ENTRY_ATTR_FILE_EXTERNAL_PATH "file-external-path"
> +#define SVN_WC__ENTRY_ATTR_FILE_EXTERNAL_REV "file-external-rev"
>
> /* Attribute values for 'schedule' */
> #define SVN_WC__ENTRY_VALUE_ADD "add"
> #define SVN_WC__ENTRY_VALUE_DELETE "delete"
> #define SVN_WC__ENTRY_VALUE_REPLACE "replace"
>
> +/* Attribute value for 'file-external' */
> +#define SVN_WC__ENTRY_VALUE_HEAD "head"
>
>
>
> /* Initialize an entries file based on URL at INITIAL_REV, in the adm
> @@ -159,6 +163,8 @@
> #define SVN_WC__ENTRY_MODIFY_CHANGELIST
> APR_INT64_C(0x0000000040000000)
> #define SVN_WC__ENTRY_MODIFY_KEEP_LOCAL
> APR_INT64_C(0x0000000080000000)
> #define SVN_WC__ENTRY_MODIFY_WORKING_SIZE
> APR_INT64_C(0x0000000100000000)
> +#define SVN_WC__ENTRY_MODIFY_FILE_EXTERNAL_PATH
> APR_INT64_C(0x0000000200000000)
> +#define SVN_WC__ENTRY_MODIFY_FILE_EXTERNAL_REV
> APR_INT64_C(0x0000000400000000)
> /* No #define for DEPTH, because it's only meaningful on this-dir anyway.
> */
>
> /* ...ORed together with this to mean "I really mean this, don't be
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>
>
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-06 19:14:53 CEST