oxigraph / rio

RDF parsers library
Apache License 2.0
87 stars 10 forks source link

please upgrade use of crate quick-xml to v0.36, to match oxigraph #142

Open jonassmedegaard opened 2 weeks ago

jonassmedegaard commented 2 weeks ago

This project uses crate quick-xml v0.28. The quick-xml project has seen a slew of bugfixes, and oxigraph has also shifted to using a newer release v0.36 in its newest release v0.4.0-alpha8.

(less of a concern for you, perhaps, but for me in packaging for Debian there is also an issue of wanting to limit the amount of concurrent versions of each project)

I am not strong at coding rust, but have tried to provide a patch:

--- a/xml/Cargo.toml
+++ b/xml/Cargo.toml
@@ -22,4 +22,4 @@
 oxilangtag = "0.1"
 oxiri = "0.2"
 rio_api = { version = "0.8", path="../api" }
-quick-xml = "0.28"
+quick-xml = ">= 0.28, <= 0.36"
--- a/xml/src/error.rs
+++ b/xml/src/error.rs
@@ -99,7 +99,7 @@
         match error.kind {
             RdfXmlErrorKind::Xml(error) => match error {
                 quick_xml::Error::Io(error) => io::Error::new(error.kind(), error),
-                quick_xml::Error::UnexpectedEof(error) => {
+                quick_xml::Error::IllFormed(error) => {
                     io::Error::new(io::ErrorKind::UnexpectedEof, error)
                 }
                 error => io::Error::new(io::ErrorKind::InvalidData, error),
--- a/xml/src/parser.rs
+++ b/xml/src/parser.rs
@@ -63,7 +63,7 @@
     /// Builds the parser from a `BufRead` implementation, and a base IRI for relative IRI resolution.
     pub fn new(reader: R, base_iri: Option<Iri<String>>) -> Self {
         let mut reader = NsReader::from_reader(reader);
-        reader.expand_empty_elements(true);
+        reader.config_mut().expand_empty_elements = true;
         Self {
             reader: RdfXmlReader {
                 reader,
@@ -79,7 +79,7 @@
     }

     /// The current byte position in the input data.
-    pub fn buffer_position(&self) -> usize {
+    pub fn buffer_position(&self) -> u64 {
         self.reader.reader.buffer_position()
     }
 }
@@ -1021,7 +1021,7 @@

     fn convert_attribute(&self, attribute: Attribute) -> Result<String, RdfXmlError> {
         Ok(attribute
-            .decode_and_unescape_value_with(&self.reader, |e| self.resolve_entity(e))?
+            .decode_and_unescape_value_with(self.reader.decoder(), |e| self.resolve_entity(e))?
             .to_string())
     }

With the patch applied, the build succeeds but one test fails:

 Running tests/roundtrip.rs (target/x86_64-unknown-linux-gnu/debug/deps/roundtrip-311d9100fcca5190)

running 1 test test simple_roundtrip ... FAILED

failures:

---- simple_roundtrip stdout ---- Error: RdfXmlError { kind: Xml(EscapeError(UnrecognizedEntity(4..8, "quot"))) }

failures: simple_roundtrip

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Tpt commented 2 weeks ago

@jonassmedegaard Hey! rio is not used by Oxigraph anymore. Do you still need to package it in Debian? if yes, I am happy to update its dependencies.

jonassmedegaard commented 2 weeks ago

I am working on packaging Atomic Server, Sophia, Manas, all still using rio_* crates, as far as I am aware (but that said, I had missed that Oxigraph had moved away from those crates - thanks for bringing it to my attention, I've corrected that just now - sp I migh be mistaken about some of the others as well.

So if not too much trouble, I would indeed appreciate your modernizing the rdf_xml crate - and while at it, it you consider them deprecated then perhaps make that explicit by mentioning it at the top of the README.md fine (and whatever cargo tagging might be relevant as well).

Tpt commented 2 weeks ago

Done v0.8.5

I plan to maintain the rio crates for a year after the first stable release of oxttl and oxrdfxml

Rio has been used quite a lot in the wild, I do not expect many bug reports at this point.