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

Fail to compile on android because IndexMap #647

Open
mamcx opened this issue Aug 10, 2023 · 4 comments
Open

Fail to compile on android because IndexMap #647

mamcx opened this issue Aug 10, 2023 · 4 comments

Comments

@mamcx
Copy link

mamcx commented Aug 10, 2023

I start to get breaking CI with:

ureq = { version = "2.7.1", features = ["default", "json", "charset", "cookies"] }

With error:

error[E0107]: struct takes 3 generic arguments but 2 generic arguments were supplied
  --> /Users/mamcx/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cookie_store-0.19.0/src/cookie_store.rs:23:18
   |
23 | type Map<K, V> = IndexMap<K, V>;
   |                  ^^^^^^^^ -  - supplied 2 generic arguments
   |                  |
   |                  expected 3 generic arguments
   |
note: struct defined here, with 3 generic parameters: `K`, `V`, `S`
  --> /Users/mamcx/.cargo/registry/src/index.crates.io-6f17d22bba15001f/indexmap-1.9.2/src/map.rs:76:12
   |
76 | pub struct IndexMap<K, V, S> {
   |            ^^^^^^^^ -  -  -
help: add missing generic argument
   |
23 | type Map<K, V> = IndexMap<K, V, S>;
   |                               +++
@algesten
Copy link
Owner

This sounds like a breakage that is beyond ureq's control. Ureq doesn't depend on indexmap, but cookie-store does.

├── cookie_store v0.19.1
│   ├── cookie v0.16.2 (*)
│   ├── idna v0.3.0
│   │   ├── unicode-bidi v0.3.13
│   │   └── unicode-normalization v0.1.22
│   │       └── tinyvec v1.6.0
│   │           └── tinyvec_macros v0.1.1
│   ├── indexmap v1.9.3
│   │   └── hashbrown v0.12.3
│   │   [build-dependencies]
│   │   └── autocfg v1.1.0

It seems either something broke in indexmap, or your project is pulling in a version of indexmap that cookie_store isn't happy about.

We need to update cookie_store in ureq, so maybe that solves it for you also.

@pfernie
Copy link

pfernie commented Aug 12, 2023

Hi there! First, for clarity the indexmap dependency is only drawn in when the preserve_order feature of cookie_store is enabled. This enables the behavior that cookies are iterated in insertion order; I believe in principle cookie ordering should not be relied on, but in practice some servers do (hence the feature). So perhaps that is the desired out-of-box experience for ureq, but one option would be providing more features or different defaults.

Ultimately, it looks like this breakage is due to a feature gate related to std in indexmap. It seems prior to 2.x, has_std detection was done in build.rs. This behavior was changed for the 2.x series, perhaps due to issues like these?

I am not too familiar with no_std scenarios, nor building for android. I have pushed a commit bumping the indexmap dependency in cookie_store, so you may try to put in a dependency patch and build with that version to see if it addresses the build issue.

@mamcx
Copy link
Author

mamcx commented Aug 25, 2023

How can I do a dependency patch?

I tried with creating a simple bin:

[package]
name = "TestUreq"
version = "0.1.0"
edition = "2021"

[dependencies]
ureq = {git = "https://github.com/algesten/ureq", features = ["default", "json", "charset", "cookies"] }

[patch.crates-io]
cookie_store = { git = "https://github.com/pfernie/cookie_store.git", rev ="6006e02" }

but get the same error:

 cargo +nightly ndk --target=armeabi-v7a build -Zbuild-std

error[E0107]: struct takes 3 generic arguments but 2 generic arguments were supplied
  --> /Users/mamcx/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cookie_store-0.20.0/src/cookie_store.rs:23:18
   |
23 | type Map<K, V> = IndexMap<K, V>;
   |                  ^^^^^^^^ -  - supplied 2 generic arguments
   |                  |
   |                  expected 3 generic arguments
   |
note: struct defined here, with 3 generic parameters: `K`, `V`, `S`
  --> /Users/mamcx/.cargo/registry/src/index.crates.io-6f17d22bba15001f/indexmap-1.9.3/src/map.rs:76:12
   |
76 | pub struct IndexMap<K, V, S> {
   |            ^^^^^^^^ -  -  -
help: add missing generic argument
   |
23 | type Map<K, V> = IndexMap<K, V, S>;
   |                               +++

For more information about this error, try `rustc --explain E0107`.

@pfernie
Copy link

pfernie commented Aug 27, 2023

This was my fault; I did not remember that [patch] directives will still respect semver. Since I changed the numbering on the test commit to 0.21.0-pre, it wasn't taken by your [patch] setup. I've pushed a new commit which changes the numbering to 0.20.1, which should work for you.

A test project with this Cargo.toml:

[package]
name = "test-ureq"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
ureq = { git = "https://github.com/algesten/ureq", features = ["default", "json", "charset", "cookies" ], rev = "50fd1fe" }

[patch.crates-io]
cookie_store = { git = "https://github.com/pfernie/cookie_store", rev = "9eed31b" }

Successfully pulls in the updated dependency:

   Compiling cookie_store v0.20.1 (https://github.com/pfernie/cookie_store?rev=9eed31b#9eed31b9)
   Compiling ureq v2.7.1 (https://github.com/algesten/ureq?rev=50fd1fe#50fd1fe1)
   Compiling test-ureq v0.1.0 (/home/patrick/src/scratch/test-ureq)

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