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

Re: [PATCH] 'svn info --xml' - v5

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-08-02 23:12:06 CEST

Alexander Thomas wrote:
>
> Patch to add XML output for 'svn info'.

I'm testing this patch against its DTD by running the following command in my
Subversion working copy and in my "sandbox" working copy (which has all sorts
of odd things in it):

$ svn info --xml -R * | xmllint --dtdvalid
~/src/subversion/subversion/clients/cmdline/dtd/info.dtd --noout -

I've also been looking at the other DTDs (blame, list, log, status) to see how
this compares with them, and looking at the source of the information
(svn_info_t) to see what it really means and which bits are related to each other.

In deciding which elements are optional and which are dependent on others, I
could do with some help from somebody who understands the client and WC better
than me. To me, it seems that nearly everything is optional, and it seems that
this has only been determined empirically by finding test cases in which some
bit of information is not present.

One further thing I'd like to see, but that can be done afterwards, is the DTDs
(all of them) better laid out, with replaceable entities naming data types like
"pathname", "username", "date", etc., even though all of those are just defined
as "(#PCDATA)"; and better comments that state both the meaning and the data
type (format) of each field.

[...]
> (svn_cl__node_kind_str): New prototype.
[...]
> (make_tagged_cdata): New function.

I decided to add these both as semi-public functions, and have committed them
already in r15545, the latter named "svn_cl__xml_tagged_cdata".

> Index: subversion/clients/cmdline/dtd/info.dtd
> ===================================================================
> --- subversion/clients/cmdline/dtd/info.dtd (revision 0)
> +++ subversion/clients/cmdline/dtd/info.dtd (revision 0)
> @@ -0,0 +1,47 @@
> +<!-- XML DTD for Subversion command-line client output. -->
> +
> +<!-- For "svn info" -->
> +<!ELEMENT info (entry*)>
> +
> +<!ELEMENT entry (url?, repository?, wc-info?, last-changed?, conflict?, lock?)>
> +<!-- path: local path -->
> +<!-- kind: path type -->
> +<!-- revision number of wc or repos path/url-->
> +<!ATTLIST entry
> + path CDATA #REQUIRED
> + kind (file | directory) #REQUIRED

I've changed "directory" to "dir" for consistency with "svn list --xml".

> + revision CDATA #REQUIRED
> +>
> +<!ELEMENT url (#PCDATA)> <!-- repository url -->
> +
> +<!ELEMENT repository (root?, uuid)>

It seems that the UUID needs to be optional. I have some directories that have
both root and uuid, and some with only root, and some with only uuid.

> +<!ELEMENT root (#PCDATA)> <!-- root: repository URL -->
> +<!ELEMENT uuid (#PCDATA)> <!-- uuid: repository uuid -->
> +
> +<!ELEMENT wc-info (schedule?, copy-from-url?, copy-from-rev?, checksum?)>
> +<!-- schedule: applies only for wc -->

Hmm. All of the "wc-info" fields apply only to a wc.

> +<!ELEMENT schedule (#PCDATA | normal | add | delete | replace | none)*>

This implies mixed content. I've changed this to:
<!ELEMENT schedule (#PCDATA)> <!-- normal | add | delete | replace | none -->

> +<!ELEMENT copy-from-url (#PCDATA)>
> +<!ELEMENT copy-from-rev (#PCDATA)>

The code was writing "copied-from-rev". I've fixed the code.

> +<!ELEMENT checksum (#PCDATA)>
> +
> +<!ELEMENT last-changed (author, revision, date, text-updated?, prop-updated?)>
> +<!ELEMENT author (#PCDATA)> <!-- last changed author -->
> +<!ELEMENT revision (#PCDATA)> <!-- last changed revision -->

This "last-changed" element ought to be consistent with the "commit" element of
"status.dtd" and "list.dtd", even as far as being named "commit".

> +<!ELEMENT date (#PCDATA)> <!-- last changed date in iso format -->
> +<!ELEMENT text-updated (#PCDATA)> <!-- text last updated -->
> +<!ELEMENT prop-updated (#PCDATA)> <!-- properties last updated -->

Those last two elements are WC info, not last-changed info. See the definition
of svn_info_t for details.

> +<!ELEMENT conflict (prev-base-file, prev-wc-file?, cur-base-file, prop-file?)>
> +<!ELEMENT prev-base-file (#PCDATA)> <!-- previous base file -->
> +<!ELEMENT prev-wc-file (#PCDATA)> <!-- previous wc file -->
> +<!ELEMENT cur-base-file (#PCDATA)> <!-- current base file -->
> +<!ELEMENT prop-file (#PCDATA)> <!-- current properties file -->

This conflict information is also a subset of the WC information.

> +<!ELEMENT lock (token, owner, created, expires?, comment?)>
> +<!ELEMENT token (#PCDATA)> <!-- lock token -->
> +<!ELEMENT owner (#PCDATA)> <!-- lock owner -->
> +<!ELEMENT created (#PCDATA)> <!-- lock created: date & time in iso format -->
> +<!ELEMENT expires (#PCDATA)> <!-- lock expires: date & time in iso format -->
> +<!ELEMENT comment (#PCDATA)> <!-- lock comment -->

This "lock" element should be consistent with the one in "status.dtd"; it very
nearly is, but the order of the elements is different.

[...]
> + /* "<copied-from-rev> xx </copied-from-rev>" */
> + make_tagged_cdata (&sb, pool, "copied-from-rev",

I've corrected this from "copied-from-rev" to "copy-from-rev".

> + apr_psprintf (pool, "%ld", info->copyfrom_rev));

[...]
> @@ -76,26 +315,9 @@
> if (SVN_IS_VALID_REVNUM (info->rev))
> SVN_ERR (svn_cmdline_printf (pool, _("Revision: %ld\n"), info->rev));
>
> - switch (info->kind)
> - {
> - case svn_node_file:
> - SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: file\n")));
> - break;
> -
> - case svn_node_dir:
> - SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: directory\n")));
> - break;
> -
> - case svn_node_none:
> - SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: none\n")));
> - break;
> -
> - case svn_node_unknown:
> - default:
> - SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: unknown\n")));
> - break;
> - }
>
> + SVN_ERR (svn_cmdline_printf (pool, _("Node Kind: %s\n"),
> + svn_cl__node_kind_str (info->kind)));

As I said, we probably want those strings to remain localisable, while the XML
output is fixed, so we'll leave that part of the code as it was, and not call
the new "node_kind_str" function there.

So, please find attached my version of your patch after making all of the above
changes.

I think this is ready to commit as far I am concerned, but I would really like
someone else to review particularly the format of the XML if possible. If
there are no further concerns in the next two or three days I will commit this.

Please say if you don't agree with something I said or did, or if you would
like to make further changes.

- Julian

[[[
Add XML output for "svn info".

Patch by: Alexander Thomas <alexander@collab.net>
(Tweaked by me.)

* subversion/clients/cmdline/dtd/info.dtd
  New DTD.

* subversion/clients/cmdline/main.c
  (svn_cl__cmd_table): Add "--xml" and "--incremental" options for "svn info".

* subversion/clients/cmdline/info-cmd.c
  (print_header_xml): New function.
  (print_footer_xml): New function.
  (schedule_str): New function.
  (print_info_xml): New function.
  (info_receiver): Print XML or traditional output depending on cmdline option.
  (svn_cl__info): Modified for XML output.

* tools/client-side/bash_completion
  (_svn): Add "--xml" and "--incremental" options for "svn info".
]]]

Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c (revision 15544)
+++ subversion/clients/cmdline/main.c (working copy)
@@ -371,8 +371,8 @@
        "\n"
        " Print information about each TARGET (default: '.')\n"
        " TARGET may be either a working-copy path or URL.\n"),
- {'r', 'R', svn_cl__targets_opt, SVN_CL__AUTH_OPTIONS,
- svn_cl__config_dir_opt} },
+ {'r', 'R', svn_cl__targets_opt, svn_cl__incremental_opt, svn_cl__xml_opt,
+ SVN_CL__AUTH_OPTIONS, svn_cl__config_dir_opt} },
  
   { "list", svn_cl__ls, {"ls"},
     N_("List directory entries in the repository.\n"
Index: subversion/clients/cmdline/dtd/info.dtd
===================================================================
--- subversion/clients/cmdline/dtd/info.dtd (revision 0)
+++ subversion/clients/cmdline/dtd/info.dtd (revision 0)
@@ -0,0 +1,50 @@
+<!-- XML DTD for Subversion command-line client output. -->
+
+<!-- Common attributes and elements -->
+<!ELEMENT author (#PCDATA)> <!-- user name -->
+<!ELEMENT date (#PCDATA)> <!-- date as "yyyy-mm-ddThh:mm:ss.ssssssZ" -->
+
+<!-- For "svn info" -->
+<!ELEMENT info (entry*)>
+
+<!ELEMENT entry (url?, repository?, wc-info?, commit?, lock?)>
+<!-- path: local path -->
+<!-- kind: path type -->
+<!-- revision number of wc or repos path/url-->
+<!ATTLIST entry
+ path CDATA #REQUIRED
+ kind (file | dir) #REQUIRED
+ revision CDATA #REQUIRED
+>
+
+<!ELEMENT url (#PCDATA)> <!-- URL of this item in the repository -->
+
+<!ELEMENT repository (root?, uuid?)>
+<!ELEMENT root (#PCDATA)> <!-- root: URL of the repository -->
+<!ELEMENT uuid (#PCDATA)> <!-- uuid: UUID of the repository -->
+
+<!ELEMENT wc-info (schedule?, copy-from-url?, copy-from-rev?,
+ text-updated?, prop-updated?, checksum?, conflict?)>
+<!ELEMENT schedule (#PCDATA)> <!-- normal | add | delete | replace | none -->
+<!ELEMENT copy-from-url (#PCDATA)>
+<!ELEMENT copy-from-rev (#PCDATA)>
+<!ELEMENT text-updated (#PCDATA)> <!-- date text last updated -->
+<!ELEMENT prop-updated (#PCDATA)> <!-- date properties last updated -->
+<!ELEMENT checksum (#PCDATA)>
+
+<!ELEMENT conflict (prev-base-file, prev-wc-file?, cur-base-file, prop-file?)>
+<!ELEMENT prev-base-file (#PCDATA)> <!-- previous base file -->
+<!ELEMENT prev-wc-file (#PCDATA)> <!-- previous wc file -->
+<!ELEMENT cur-base-file (#PCDATA)> <!-- current base file -->
+<!ELEMENT prop-file (#PCDATA)> <!-- current properties file -->
+
+<!-- The last commit, or the "base" revision for a WC item -->
+<!ELEMENT commit (author?, date?)>
+<!ATTLIST commit revision CDATA #REQUIRED> <!-- revision number: integer -->
+
+<!ELEMENT lock (token, owner, comment?, created, expires?)>
+<!ELEMENT token (#PCDATA)> <!-- lock token URI -->
+<!ELEMENT owner (#PCDATA)> <!-- lock owner -->
+<!ELEMENT comment (#PCDATA)> <!-- lock comment -->
+<!ELEMENT created (#PCDATA)> <!-- creation date in ISO format -->
+<!ELEMENT expires (#PCDATA)> <!-- expiration date in ISO format -->
Index: subversion/clients/cmdline/info-cmd.c
===================================================================
--- subversion/clients/cmdline/info-cmd.c (revision 15544)
+++ subversion/clients/cmdline/info-cmd.c (working copy)
@@ -28,6 +28,7 @@
 #include "svn_error.h"
 #include "svn_path.h"
 #include "svn_time.h"
+#include "svn_xml.h"
 #include "cl.h"
 
 #include "svn_private_config.h"
@@ -47,6 +48,212 @@
   return SVN_NO_ERROR;
 }
 
+/* Prints XML header */
+static svn_error_t *
+print_header_xml (apr_pool_t *pool)
+{
+ svn_stringbuf_t *sb = svn_stringbuf_create ("", pool);
+ /* <?xml version="1.0" encoding="utf-8"?> */
+ svn_xml_make_header (&sb, pool);
+
+ /* "<info>" */
+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "info", NULL);
+
+ return svn_cl__error_checked_fputs (sb->data, stdout);
+}
+
+
+/* Prints XML footer */
+static svn_error_t *
+print_footer_xml (apr_pool_t *pool)
+{
+ svn_stringbuf_t *sb = svn_stringbuf_create ("", pool);
+ /* "</info>" */
+ svn_xml_make_close_tag (&sb, pool, "info");
+ return svn_cl__error_checked_fputs (sb->data, stdout);
+}
+
+
+/* Return string representation of SCHEDULE */
+static const char *
+schedule_str (svn_wc_schedule_t schedule)
+{
+ switch (schedule)
+ {
+ case svn_wc_schedule_normal:
+ return "normal";
+ case svn_wc_schedule_add:
+ return "add";
+ case svn_wc_schedule_delete:
+ return "delete";
+ case svn_wc_schedule_replace:
+ return "replace";
+ default:
+ return "none";
+ }
+}
+
+
+/* prints svn info in xml mode to standard out */
+static svn_error_t *
+print_info_xml (const char *target,
+ const svn_info_t *info,
+ apr_pool_t *pool)
+{
+ svn_stringbuf_t *sb = svn_stringbuf_create ("", pool);
+ const char *rev_str;
+
+ /* If revision is invalid, assume WC is corrupt. */
+ if (SVN_IS_VALID_REVNUM(info->rev))
+ rev_str = apr_psprintf (pool, "%ld", info->rev);
+ else
+ return svn_error_createf (SVN_ERR_WC_CORRUPT, NULL,
+ _("'%s' has invalid revision"),
+ svn_path_local_style (target, pool));
+
+ /* "<entry ...>" */
+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "entry",
+ "path", svn_path_local_style (target, pool),
+ "kind", svn_cl__node_kind_str (info->kind),
+ "revision", rev_str,
+ NULL);
+
+ svn_cl__xml_tagged_cdata (&sb, pool, "url", info->URL);
+
+ if (info->repos_root_URL || info->repos_UUID)
+ {
+ /* "<repository>" */
+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "repository", NULL);
+
+ /* "<root> xx </root>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "root", info->repos_root_URL);
+
+ /* "<uuid> xx </uuid>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "uuid", info->repos_UUID);
+
+ /* "</repository>" */
+ svn_xml_make_close_tag (&sb, pool, "repository");
+ }
+
+ if (info->has_wc_info)
+ {
+ /* "<wc-info>" */
+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "wc-info", NULL);
+
+ /* "<schedule> xx </schedule>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "schedule",
+ schedule_str (info->schedule));
+
+ /* "<copy-from-url> xx </copy-from-url>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "copy-from-url",
+ info->copyfrom_url);
+
+ /* "<copy-from-rev> xx </copy-from-rev>" */
+ if (SVN_IS_VALID_REVNUM (info->copyfrom_rev))
+ svn_cl__xml_tagged_cdata (&sb, pool, "copy-from-rev",
+ apr_psprintf (pool, "%ld",
+ info->copyfrom_rev));
+
+ /* "<text-updated> xx </text-updated>" */
+ if (info->text_time)
+ svn_cl__xml_tagged_cdata (&sb, pool, "text-updated",
+ svn_time_to_cstring (info->text_time, pool));
+
+ /* "<prop-updated> xx </prop-updated>" */
+ if (info->prop_time)
+ svn_cl__xml_tagged_cdata (&sb, pool, "prop-updated",
+ svn_time_to_cstring (info->prop_time, pool));
+
+ /* "<checksum> xx </checksum>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "checksum", info->checksum);
+
+ /* "</wc-info>" */
+ svn_xml_make_close_tag (&sb, pool, "wc-info");
+ }
+
+ if (info->last_changed_author
+ || SVN_IS_VALID_REVNUM (info->last_changed_rev)
+ || info->last_changed_date)
+ {
+ /* "<commit ...>" */
+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "commit",
+ "revision", apr_psprintf (pool, "%ld",
+ info->last_changed_rev),
+ NULL);
+
+ /* "<author> xx </author>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "author",
+ info->last_changed_author);
+
+ /* "<date> xx </date>" */
+ if (info->last_changed_date)
+ svn_cl__xml_tagged_cdata (&sb, pool, "date",
+ svn_time_to_cstring
+ (info->last_changed_date, pool));
+
+ /* "</commit>" */
+ svn_xml_make_close_tag (&sb, pool, "commit");
+ }
+
+ if (info->conflict_old || info->conflict_wrk
+ || info->conflict_new || info->prejfile)
+ {
+ /* "<conflict>" */
+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "conflict", NULL);
+
+ /* "<prev-base-file> xx </prev-base-file>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "prev-base-file",
+ info->conflict_old);
+
+ /* "<prev-wc-file> xx </prev-wc-file>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "prev-wc-file",
+ info->conflict_wrk);
+
+ /* "<cur-base-file> xx </cur-base-file>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "cur-base-file",
+ info->conflict_new);
+
+ /* "<prop-file> xx </prop-file>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "prop-file", info->prejfile);
+
+ /* "</conflict>" */
+ svn_xml_make_close_tag (&sb, pool, "conflict");
+ }
+
+ if (info->lock)
+ {
+ /* "<lock>" */
+ svn_xml_make_open_tag (&sb, pool, svn_xml_normal, "lock", NULL);
+
+ /* "<token> xx </token>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "token", info->lock->token);
+
+ /* "<owner> xx </owner>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "owner", info->lock->owner);
+
+ /* "<comment ...> xxxx </comment>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "comment", info->lock->comment);
+
+ /* "<created> xx </created>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "created",
+ svn_time_to_cstring
+ (info->lock->creation_date, pool));
+
+ /* "<expires> xx </expires>" */
+ svn_cl__xml_tagged_cdata (&sb, pool, "expires",
+ svn_time_to_cstring
+ (info->lock->expiration_date, pool));
+
+ /* "</lock>" */
+ svn_xml_make_close_tag (&sb, pool, "lock");
+ }
+
+ /* "</entry>" */
+ svn_xml_make_close_tag (&sb, pool, "entry");
+
+ return svn_cl__error_checked_fputs (sb->data, stdout);
+}
+
 
 static svn_error_t *
 print_info (const char *target,
@@ -217,7 +424,6 @@
 }
 
 
-
 /* A callback of type svn_info_receiver_t. */
 static svn_error_t *
 info_receiver (void *baton,
@@ -225,11 +431,13 @@
                const svn_info_t *info,
                apr_pool_t *pool)
 {
- return print_info (path, info, pool);
+ if (((svn_cl__cmd_baton_t *) baton)->opt_state->xml)
+ return print_info_xml (path, info, pool);
+ else
+ return print_info (path, info, pool);
 }
 
 
-
 /* This implements the `svn_opt_subcommand_t' interface. */
 svn_error_t *
 svn_cl__info (apr_getopt_t *os,
@@ -249,6 +457,22 @@
 
   /* Add "." if user passed 0 arguments. */
   svn_opt_push_implicit_dot_target (targets, pool);
+
+ if (opt_state->xml)
+ {
+ /* If output is not incremental, output the XML header and wrap
+ everything in a top-level element. This makes the output in
+ its entirety a well-formed XML document. */
+ if (! opt_state->incremental)
+ SVN_ERR (print_header_xml (pool));
+ }
+ else
+ {
+ if (opt_state->incremental)
+ return svn_error_create (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("'incremental' option only valid in XML "
+ "mode"));
+ }
   
   for (i = 0; i < targets->nelts; i++)
     {
@@ -268,7 +492,7 @@
 
       err = svn_client_info (truepath,
                              &peg_revision, &(opt_state->start_revision),
- info_receiver, NULL,
+ info_receiver, baton,
                              opt_state->recursive, ctx, subpool);
 
       /* If one of the targets is a non-existent URL or wc-entry,
@@ -297,5 +521,8 @@
     }
   svn_pool_destroy (subpool);
 
+ if (opt_state->xml && (! opt_state->incremental))
+ SVN_ERR (print_footer_xml (pool));
+
   return SVN_NO_ERROR;
 }
Index: tools/client-side/bash_completion
===================================================================
--- tools/client-side/bash_completion (revision 15544)
+++ tools/client-side/bash_completion (working copy)
@@ -99,7 +99,8 @@
                          --editor-cmd $pOpts"
                 ;;
         info)
- cmdOpts="$pOpts $rOpts --targets -R --recursive"
+ cmdOpts="$pOpts $rOpts --targets -R --recursive \
+ --incremental --xml"
                 ;;
         list|ls)
                 cmdOpts="$rOpts -v --verbose -R --recursive $pOpts \

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 2 23:15:12 2005

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.