search

sagebind / isahc

The practical HTTP client that is fun to use.
https://docs.rs/isahc
MIT License
711 stars 62 forks source link

tests fail on architectures without AtomicU64 #423

Closed jonassmedegaard closed 1 year ago

jonassmedegaard commented 1 year ago

In my packaging isahc for Debian I have learned that one of its build-dependencies - threadfin - uses AtomicU64 which is unsupported on some of the architectures otherwise supported for Rust on Debian - armel (a.k.a. armv5te) and mipsel (and possibly also s390x).

If not too much trouble to switch to a different threadpool crate without that constraint, then that would ease maintenance of the distribution for Debian.

Alternatively, you might want to consider skipping testserver-related build and testing. I am not strong in coding Rust but perhaps my crude attempt at doing that can be of interest - available here: https://salsa.debian.org/debian/rust-isahc/-/blob/debian/1.7.2+ds-10/debian/patches/2004_testserver_limit_archs.patch

sagebind commented 1 year ago

Thanks for opening an issue! I must admit that I am surprised though that you would want to build the testserver when creating a Debian package, as it is only used for tests. Do you build and run tests then as part of the packaging process? I'm not too familiar with the Debian packaging workflow.

Since I'm a maintainer of Threadfin too, I think probably the easiest solution would be to get Threadfin updated so that it will compile on those architectures.

sagebind commented 1 year ago

Also just briefly looking at your patch, commenting out those parts of testserver will likely cause a bunch of tests to fail as they are required for a number of the tests that use testserver.

jonassmedegaard commented 1 year ago

Debian has build-time tests and CI tests. Debian distributes both "source packages" (equivalent to a release or snapshot of a Github repo) and "binary packages" (the out put of configure; make; DESTDIR=local-dir make install or cargo install --root local-dir). build-time tests are executed each time a new source package is released and a set of binary packages are generated, and CI tests are executed at random.

My patch is possibly totally useless: I realized after filing this issue that since cargo resolves dependencies across all target architectures, there is likely no real benefit in the kind of hints I did in my patch because every build on any architecture will need all dependencies of all architectures installed anyway.

...so I am happy that you acknowledge this as an issue and will try address it upstream :-)

jonassmedegaard commented 1 year ago

For the record, my patch turning out to likely be impossible to succeed should probably not be taken as an indication that there is anything wrong in your upstream dependency chain. Instead, it is more a clash between a lack of flexibility of the cargo resolver (which I guess is too little of a benefit in the general Rust ecosystem to be a high priority to fix) and a special oddity of Debian packaging of Rust crates: Debian distinguishes between architecture-independent and architecture-specific binary packages, and packaging the source Rust code of a cargo crate would is architecture-independent in nature but currently we "cheat" and package them as architecture-specific due to a limitation of Debian infrastructure (we can rebuild new binary packages from same source package only for architecture-specific packages, whereas for arch-independent packages a rebuild requires a new manually released source package). If I didn't use the "trick" but packaged threadfin as arch-independent, then failure to build that on some archs would not cause it to be unavailable on all archs.

Sorry for the details - summary is that you no doubt want best possible coverate, but Debian pains are only partly caused by this, so there are other options at my disposal if you cannot offer full coverage :-)

jonassmedegaard commented 1 year ago

This issue is solved in threadfin v0.1.2.