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

chore/fix: add CI for more platforms and targets, fix build error on 32bit systems #46

Merged
merged 19 commits into from
Aug 19, 2024

Conversation

Frando
Copy link
Contributor

@Frando Frando commented Aug 13, 2024

This adds more CI workflows (adapted from iroh):

  • Build and test on mac OS
  • Build and test on windows
  • Build and test on android (aarch64 and armv7)
  • Cross-compile and test on i686 (32bit) linux, armv7 linux, aarch64 linux
  • Run cargo fmt and cargo clippy checks
  • Use nextest for running tests (runs test in parallel, much faster)
  • Runs only fuzz tests on nightly, all others on stable

apart from the CI workflow this

  • fixes build failures on 32bit systems by using u64 not usize in more places
  • tries to fix some unused import warnings wrt syncify (but failed, added an allow - will have to be revisited)

All green now!

@Frando Frando marked this pull request as ready for review August 13, 2024 09:03
@Frando Frando changed the title chore: add CI for windows, android, and 32bit cross compile chore/fix: add CI for more platforms and targets, fix build error on 32bit systems Aug 13, 2024
@AljoschaMeyer
Copy link
Contributor

tries to fix some unused import warnings wrt syncify (but failed, added an allow - will have to be revisited)

This is due to how syncify works: you place those syncify_replace annotations on a use, but they get copied into the non-syncified code (i.e., the original version of the code gets copied completely untouched, retaining those annotations).

The principled solution would be to modify the syncify macro to drop all syncify_replace annotations in its async output. So running a VisitMut over input_mod just before line 27 here. I can add that some time later (might be up to two weeks till I can do proper coding again).

@Frando
Copy link
Contributor Author

Frando commented Aug 14, 2024

This is due to how syncify works: you place those syncify_replace annotations on a use, but they get copied into the non-syncified code (i.e., the original version of the code gets copied completely untouched, retaining those annotations).

Right, makes sense to me now. So for now the allow is the correct solution then, and we can remove it once syncify removes the import (but not a priority IMO).

@AljoschaMeyer
Copy link
Contributor

AljoschaMeyer commented Aug 14, 2024

Re Ord on Entry again: the sideloading protocol defines a total order on entries to use (entries "being prior" to others): https://willowprotocol.org/specs/sideloading/index.html#sideload_protocol

The implementation of Ord should implement that order, and refer to it in the doc comment. Sorry, I forgot about that order earlier.

Copy link
Contributor

@sgwilym sgwilym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is really great to have coverage like this, thank you!

.github/workflows/rust.yml Show resolved Hide resolved
@sgwilym sgwilym merged commit e30304e into earthstar-project:main Aug 19, 2024
14 checks passed
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

Successfully merging this pull request may close these issues.

3 participants