Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XML parser doesn't support namespace declarations #307

Open
m8pple opened this issue Jan 13, 2022 · 4 comments
Open

XML parser doesn't support namespace declarations #307

m8pple opened this issue Jan 13, 2022 · 4 comments

Comments

@m8pple
Copy link
Contributor

m8pple commented Jan 13, 2022

The XML parser does not correctly process and ignore namespace declarations. e.g.:

<?xml version='1.0'?>
<Graphs xmlns="https://poets-project.org/schemas/virtual-graph-schema-v4" formatMinorVersion="0">
<GraphType xmlns="https://poets-project.org/schemas/virtual-graph-schema-v4" xmlns:ns0="https://poets-project.org/schemas/virtual-graph-schema-v3" id="clock_tree">
  <Properties><![CDATA[uint32_t max_ticks;
]]></Properties>
  <SharedCode><![CDATA[

where "ns0" is never used as a prefix. This pattern occasionally turns up when gluing together
xml fragments in other languages, and a compliant XML parser would be expected to just ignore
the prefix (well, unless ns0 is actually used, then it should process it).

I guess it is an artefact of rolling your own XML front-end - the IC and UoN tools never had a problem
as they all used pre-existing XML parsers. But I'm not sure why it wasn't picked up as a parsing problem
during parser development, as it appears in 100s of the v4 xml graphs included in the 2019-09-06
stress/benchmark set (though not all of them, as they come from different generators).

It can be worked around on the input side with filter programs, so it probably isn't worth the
effort of fixing this in the Orchestrator - but still an issue, albeit a low-priority one.

@DrongoTheDog
Copy link

It was a deliberate decision not to include namespaces. The objective at the time was to support the minimum viable subset as fast as possible so we could move around/past/through the ADR contribution.
I couldn't (and still can't) see how these would be useful in the POETS context, but it wold be quite easy to incorporate them if anyone tells me in what form they would be stored. (Compound names -> vectors of strings?)

@DrongoTheDog
Copy link

Or I could just replace the ':' with a '_' and turn the thing into a simple name? I don't really like the idea of embedding ':' in a string, 'cos it's an operator elsewhere in the grammar and there be dragons.

@mvousden
Copy link
Contributor

Decision (DBT, ADB, GMB, MLV) to ignore xmlns entirely (at least for now).

@m8pple
Copy link
Contributor Author

m8pple commented Jul 12, 2022

This one is still relevant: there is perfectly valid v3 XML that still exists and is still used, as well
as v4 XML. Ignoring v4 XML is an appropriate strategy for the Orchestrator as there are tools
for converting vX to v4, but XML that contains no xmlns cannot be parsed.

So the Orchestrator can continue to ignore xmlns. But any example files should still contain
a valid xmlns so that other implementations can understand how to parse them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants