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

quick-xml requires implicitly enabled serde/derive #473

Closed
loyd opened this issue Sep 8, 2022 · 8 comments · Fixed by #524
Closed

quick-xml requires implicitly enabled serde/derive #473

loyd opened this issue Sep 8, 2022 · 8 comments · Fixed by #524

Comments

@loyd
Copy link

loyd commented Sep 8, 2022

It's impossible to depend on quick-xml/serialize without depending on serde/derive

Details: #472

@Mingun
Copy link
Collaborator

Mingun commented Sep 8, 2022

@loyd , could you please describe, how you faced with this problem? You tried to use quick-xml in a crate with resolver="2" field?

@loyd
Copy link
Author

loyd commented Sep 8, 2022

I've fixed fixnum to support <tag>42.42</tag> parsing (by implementing visit_map). fixnum doesn't use serde/derive.

So, now it's fixed by enabling serde/derive.

But it also requires serde/derive in dev-dependencies, but it's inconvenient and allows to fail in the future (because not all feature combinations are checked by clippy, a lot of them).

@loyd
Copy link
Author

loyd commented Sep 8, 2022

Btw, next_key() cannot parse into &str (and next_value() too until v0.24 version). But it's a subject of another issue.

@Mingun
Copy link
Collaborator

Mingun commented Sep 8, 2022

I've almost done with rewriting

Btw, next_key() cannot parse into &str (and next_value() too until v0.24 version). But it's a subject of another issue.

Yes, that is known problem

I've fixed fixnum to support <tag>42.42</tag> parsing (by implementing visit_map).

The problem with map is that deserialization performed using deserialize_any. It have many drawbacks for non-self-describing deserializers and deserializers with weak type systems. XML has only two types: string and map, and <tag>42.42</tag> is mapped to map!["$value" -> "42.42"].

Unfortunately, your code will break in one of next quick-xml release, because I plan to rename $value key to #text (actually split it into #text and #content as described in #383 (comment)).

@Mingun
Copy link
Collaborator

Mingun commented Sep 8, 2022

@loyd, can you please post exact instruction, what I should do to reproduce that? I didn't see a resolver="2" field in the fixnum Cargo.toml

@loyd
Copy link
Author

loyd commented Sep 9, 2022

The problem with map is that deserialization performed using deserialize_any. It have many drawbacks for non-self-describing deserializers and deserializers with weak type systems.

Yep, a previous version uses deserialize_str for human-readable formats. But I need to support both parsing from str and from f64, so I've changed to deserialize_any.

Unfortunately, your code will break in one of next quick-xml release, because I plan to rename $value key to #text

It's ok, just specify it in the changelog explicitly.

@loyd, can you please post exact instruction, what I should do to reproduce that? I didn't see a resolver="2" field in the fixnum Cargo.toml

resolver="2" is for facing the problem in quick-xml. So, steps (in quick-xml repo):

  • Add resolver = "2" to quick-xml (or edition = "2021").
  • Run cargo clippy --features serde

It works fine with v1 because there is serde/derive dev-dep

@xkr47
Copy link

xkr47 commented Dec 1, 2022

I can reproduce this issue with these commands in an empty directory:

cargo init
echo 'quick-xml = { version = "0.26.0", features = [ "serialize" ] }' >> Cargo.toml
cargo build

By additionally issuing the below command, cargo build succeeds.

echo 'serde = { version = "1.0", features = [ "serde_derive" ] }' >> Cargo.toml

Alternatively checking out quick-xml locally, adding the features = [ "serde_derive" ] to the serde dependency in quick-xml's Cargo.toml is another way to work around this issue.

EDIT: I now understand that the issue description already covers most of this, sorry I did not realize this earlier.

@xkr47
Copy link

xkr47 commented Dec 14, 2022

I can reproduce this issue with these commands in an empty directory:

cargo init
echo 'quick-xml = { version = "0.26.0", features = [ "serialize" ] }' >> Cargo.toml
cargo build

I can confirm that #524 fixes this.

Mingun added a commit that referenced this issue Dec 21, 2022
Fix #473: add explicit serde feature that is independent from dependency
crapStone added a commit to Calciumdibromid/CaBr2 that referenced this issue Dec 29, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [quick-xml](https://github.com/tafia/quick-xml) | dependencies | minor | `0.26.0` -> `0.27.1` |

---

### Release Notes

<details>
<summary>tafia/quick-xml</summary>

### [`v0.27.1`](https://github.com/tafia/quick-xml/blob/HEAD/Changelog.md#&#8203;0271----2022-12-28)

[Compare Source](tafia/quick-xml@v0.27.0...v0.27.1)

##### Bug Fixes

-   [#&#8203;530]: Fix an infinite loop leading to unbounded memory consumption that occurs when
    skipping events on malformed XML with the `overlapped-lists` feature active.
-   [#&#8203;530]: Fix an error in the `Deserializer::read_to_end` when `overlapped-lists`
    feature is active and malformed XML is parsed

[#&#8203;530]: tafia/quick-xml#530

### [`v0.27.0`](https://github.com/tafia/quick-xml/blob/HEAD/Changelog.md#&#8203;0270----2022-12-25)

[Compare Source](tafia/quick-xml@v0.26.0...v0.27.0)

##### New Features

-   [#&#8203;521]: Implement `Clone` for all error types. This required changing `Error::Io` to contain
    `Arc<std::io::Error>` instead of `std::io::Error` since `std::io::Error` does not implement
    `Clone`.

##### Bug Fixes

-   [#&#8203;490]: Ensure that serialization of map keys always produces valid XML names.
    In particular, that means that maps with numeric and numeric-like keys (for
    example, `"42"`) no longer can be serialized because [XML name] cannot start
    from a digit
-   [#&#8203;500]: Fix deserialization of top-level sequences of enums, like
    ```xml
    <?xml version="1.0" encoding="UTF-8"?>
    <!-- list of enum Enum { A, B, С } -->
    <A/>
    <B/>
    <C/>
    ```
-   [#&#8203;514]: Fix wrong reporting `Error::EndEventMismatch` after disabling and enabling
    `.check_end_names`
-   [#&#8203;517]: Fix swapped codes for `\r` and `\n` characters when escaping them
-   [#&#8203;523]: Fix incorrect skipping text and CDATA content before any map-like structures
    in serde deserializer, like
    ```xml
    unwanted text<struct>...</struct>
    ```
-   [#&#8203;523]: Fix incorrect handling of `xs:list`s with encoded spaces: they still
    act as delimiters, which is confirmed also by mature XmlBeans Java library
-   [#&#8203;473]: Fix a hidden requirement to enable serde's `derive` feature to get
    quick-xml's `serialize` feature for `edition = 2021` or `resolver = 2` crates

##### Misc Changes

-   [#&#8203;490]: Removed `$unflatten=` special prefix for fields for serde (de)serializer, because:

    -   it is useless for deserializer
    -   serializer was rewritten and does not require it anymore

    This prefix allowed you to serialize struct field as an XML element and now
    replaced by a more thoughtful system explicitly indicating that a field should
    be serialized as an attribute by prepending `@` character to its name
-   [#&#8203;490]: Removed `$primitive=` prefix. That prefix allowed you to serialize struct
    field as an attribute instead of an element and now replaced by a more thoughtful
    system explicitly indicating that a field should be serialized as an attribute
    by prepending `@` character to its name
-   [#&#8203;490]: In addition to the `$value` special name for a field a new `$text`
    special name was added:

    -   `$text` is used if you want to map field to text content only. No markup is
        expected (but text can represent a list as defined by `xs:list` type)
    -   `$value` is used if you want to map elements with different names to one field,
        that should be represented either by an `enum`, or by sequence of `enum`s
        (`Vec`, tuple, etc.), or by string. Use it when you want to map field to any
        content of the field, text or markup

    Refer to [documentation] for details.
-   [#&#8203;521]: MSRV bumped to 1.52.
-   [#&#8203;473]: `serde` feature that used to make some types serializable, renamed to `serde-types`
-   [#&#8203;528]: Added documentation for XML to `serde` mapping

[#&#8203;473]: tafia/quick-xml#473

[#&#8203;490]: tafia/quick-xml#490

[#&#8203;500]: tafia/quick-xml#500

[#&#8203;514]: tafia/quick-xml#514

[#&#8203;517]: tafia/quick-xml#517

[#&#8203;521]: tafia/quick-xml#521

[#&#8203;523]: tafia/quick-xml#523

[#&#8203;528]: tafia/quick-xml#528

[XML name]: https://www.w3.org/TR/xml11/#NT-Name

[documentation]: https://docs.rs/quick-xml/0.27.0/quick_xml/de/index.html#difference-between-text-and-value-special-names

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC43My4wIiwidXBkYXRlZEluVmVyIjoiMzQuNzQuMiJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Co-authored-by: crapStone <crapstone01@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1692
Reviewed-by: crapStone <crapstone@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants