[elbe-devel] [PATCH 2/2] Fix xml catalog

[John Ogness]

Hi Philipp,

Comments inline.

On 2017-06-06, Philipp Arras <philipp.arras at linutronix.de> wrote:
> Add local scheme to python environment. This change is required such that
> Elbe xml validation works also in networks behind proxy servers.
> ---
>  debian/catalog.xml             | 18 ------------------
>  debian/elbe-common.xmlcatalogs |  2 +-
>  elbepack/directories.py        |  6 ++++++
>  elbepack/xmlpreprocess.py      |  3 ++-
>  schema/catalog.xml             | 18 ++++++++++++++++++
>  5 files changed, 27 insertions(+), 20 deletions(-)
>  delete mode 100644 debian/catalog.xml
>  create mode 100644 schema/catalog.xml
>
> diff --git a/debian/catalog.xml b/debian/catalog.xml
> deleted file mode 100644
> index d9f8377..0000000
> --- a/debian/catalog.xml
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -<?xml version="1.0"?>
> -<!DOCTYPE catalog PUBLIC "-//OASIS//DTD XML Catalogs V1.0//EN"
> -  "http://www.oasis-open.org/committees/entity/release/1.0/catalog.dtd">
> -
> -<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog" prefer="public">
> -  <system systemId="http://www.w3.org/2001/03/xml.xsd"
> -	  uri="xml.xsd"/>
> -
> -  <rewriteSystem systemIdStartString="http://www.w3.org/2001/03/"
> -		 rewritePrefix="./"/>
> -
> -  <system systemId="https://www.linutronix.de/projects/Elbe/dbsfed.xsd"
> -	  uri="dbsfed.xsd"/>
> -
> -  <rewriteSystem systemIdStartString="https://www.linutronix.de/projects/Elbe"
> -		 rewritePrefix="./"/>
> -
> -</catalog>
> diff --git a/debian/elbe-common.xmlcatalogs b/debian/elbe-common.xmlcatalogs
> index b398c40..5735ea7 100644
> --- a/debian/elbe-common.xmlcatalogs
> +++ b/debian/elbe-common.xmlcatalogs
> @@ -1,4 +1,4 @@
> -local;debian/catalog.xml;/usr/share/xml/elbe-common/catalog.xml
> +local;schema/catalog.xml;/usr/share/xml/elbe-common/catalog.xml
>  root-and-package;public;https://www.linutronix.de/projects/Elbe;/usr/share/xml/elbe-common/catalog.xml
>  root-and-package;system;https://www.linutronix.de/projects/Elbe/dbsfed.xsd;/usr/share/xml/elbe-common/catalog.xml
>  root-and-package;public;http://www.w3.org/XML/1998/namespace;/usr/share/xml/elbe-common/catalog.xml
> diff --git a/elbepack/directories.py b/elbepack/directories.py
> index 4e703c4..75d59fd 100644
> --- a/elbepack/directories.py
> +++ b/elbepack/directories.py
> @@ -37,6 +37,12 @@ def init_directories(elbe_relpath):
>      else:
>          examples_dir = os.path.join (elbe_dir, "examples")
>  
> +    # Set XML catalog
> +    xmlcat = elbe_dir + "/schema/catalog.xml"
> +    if os.environ.get('XML_CATALOG_FILES') is None:
> +        os.environ['XML_CATALOG_FILES'] = xmlcat
> +    else:
> +        os.environ['XML_CATALOG_FILES'] += xmlcat

This change has two problems. First, using "+= xmlcat" will cause the
new filename to be directly appended to the old filename. Indeed, this
function is called twice and you end up with
"/my/path/schema/catalog.xml/my/path/schema/catalog.xml". I could not
find the documentation explaining what the file separator is. Some quick
tests would suggest that it is whitespace.

But the second problem is that this change causes elbe to always look
for the catalog.xml relative to the elbe binary. This works when elbe is
run from the source directory but does not when installed. I did this
change to the file _instead_ of your change and it worked for me:

--- a/elbepack/directories.py
+++ b/elbepack/directories.py
@@ -34,9 +34,10 @@ def init_directories(elbe_relpath):
 
     if elbe_exe.startswith ("/usr/bin/"):
         examples_dir = "/usr/share/doc/elbe-doc/examples"
+        os.environ['XML_CATALOG_FILES'] = "/usr/share/xml/elbe-common/catalog.xml"
     else:
         examples_dir = os.path.join (elbe_dir, "examples")
-
+        os.environ['XML_CATALOG_FILES'] = elbe_dir + "debian/catalog.xml"
 
 def get_cmdlist():
     return [ x for _, x, _ in
     iter_modules(elbepack.commands.__path__) ]

I suppose a correct solution would be to append a previous value rather
than blindly overwriting it. I did not have time to confirm the correct
separator.

>  def get_cmdlist():
>      return [ x for _, x, _ in iter_modules(elbepack.commands.__path__) ]
> diff --git a/elbepack/xmlpreprocess.py b/elbepack/xmlpreprocess.py
> index 2952510..4425bd4 100644
> --- a/elbepack/xmlpreprocess.py
> +++ b/elbepack/xmlpreprocess.py
> @@ -19,6 +19,7 @@
>  import os
>  import sys
>  import elbepack
> +from elbepack.directories import elbe_dir
>  from lxml import etree
>  from lxml.etree import XMLParser,parse
>  
> @@ -27,7 +28,7 @@ class XMLPreprocessError(Exception):
>          Exception.__init__(self, message)
>  
>  def xmlpreprocess(fname, output):
> -    schema_file = "https://www.linutronix.de/projects/Elbe/dbsfed.xsd"
> +    schema_file = elbe_dir + "/schema/dbsfed.xsd"

The changes to this file are incorrect. That is what the catalog is for.

>      parser = XMLParser(huge_tree=True)
>      schema_tree = etree.parse(schema_file)
>      schema = etree.XMLSchema(schema_tree)
> diff --git a/schema/catalog.xml b/schema/catalog.xml
> new file mode 100644
> index 0000000..d9f8377
> --- /dev/null
> +++ b/schema/catalog.xml
> @@ -0,0 +1,18 @@
> +<?xml version="1.0"?>
> +<!DOCTYPE catalog PUBLIC "-//OASIS//DTD XML Catalogs V1.0//EN"
> +  "http://www.oasis-open.org/committees/entity/release/1.0/catalog.dtd">
> +
> +<catalog xmlns="urn:oasis:names:tc:entity:xmlns:xml:catalog" prefer="public">
> +  <system systemId="http://www.w3.org/2001/03/xml.xsd"
> +	  uri="xml.xsd"/>
> +
> +  <rewriteSystem systemIdStartString="http://www.w3.org/2001/03/"
> +		 rewritePrefix="./"/>
> +
> +  <system systemId="https://www.linutronix.de/projects/Elbe/dbsfed.xsd"
> +	  uri="dbsfed.xsd"/>
> +
> +  <rewriteSystem systemIdStartString="https://www.linutronix.de/projects/Elbe"
> +		 rewritePrefix="./"/>
> +
> +</catalog>

Thanks for your work on this. This is quite a painful bug to debug.

John Ogness