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(portability): Support all platforms without 64 bit atomics #203

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

navaati
Copy link
Contributor

@navaati navaati commented Jun 25, 2024

Notably, RISC-V 32 for ESP32-C3 chips. Fixes #201. Closes #202.

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

target_has_atomic is neat! Thank you @navaati. Good work.

Do you have experience cross-compiling Rust for RISC-V? If so, want to add another target to the matrix here within this pull request?

cross-compile:
name: Cross compile
runs-on: ubuntu-latest
strategy:
matrix:
target:
- armv7-unknown-linux-gnueabihf
- powerpc-unknown-linux-gnu
- powerpc64-unknown-linux-gnu
- wasm32-unknown-unknown
steps:
- uses: actions/checkout@v2
- uses: dtolnay/rust-toolchain@stable
with:
target: ${{ matrix.target }}
# Note that this does not test the `protobuf` feature (for now). See reasoning in https://github.com/prometheus/client_rust/pull/98/.
- run: cargo check --target=${{ matrix.target }}

@navaati
Copy link
Contributor Author

navaati commented Jul 1, 2024

Ah, yes. It very much depends on the support from that Github Action you use, which I’m not super familiar with.

The target I actually use, riscv32imc-esp-espidf, is Tier 3 and I’m not sure it’ll be supported (among other things, it requires building the std lib), the closest would be riscv32imc-unknown-none-elf but it is a no-std target so it makes no sense here…

I’ll put out a commit with riscv32imc-esp-espidf in the list and we’ll see how it goes, can you allow the CI to run once it’s here ?

@mxinden
Copy link
Member

mxinden commented Jul 1, 2024

I’ll put out a commit with riscv32imc-esp-espidf in the list and we’ll see how it goes, can you allow the CI to run once it’s here ?

Yep. Will do. Thanks.

@mxinden
Copy link
Member

mxinden commented Jul 1, 2024

Also note, you will need to sign off your commits, see failing CI step.

@navaati navaati force-pushed the target_has_atomic branch 3 times, most recently from 346744b to 56e6b3c Compare July 1, 2024 19:20
CHANGELOG.md Outdated Show resolved Hide resolved
@mxinden
Copy link
Member

mxinden commented Jul 1, 2024

Just triggered a CI run.

@navaati
Copy link
Contributor Author

navaati commented Jul 1, 2024

Just triggered a CI run.

Thanks. And it fails, as I feared the target needs the rust-std component and it’s nightly only… But you have powerpc and wasm32 which are 32-bit platforms, so it should be ok.

Should I just drop that CI commit and we go as is ?

Notably, RISC-V 32 for ESP32-C3 chips.

Signed-off-by: Léo Gillot-Lamure <[email protected]>
@navaati navaati force-pushed the target_has_atomic branch from 56e6b3c to 4129bb0 Compare July 1, 2024 20:06
@navaati
Copy link
Contributor Author

navaati commented Jul 1, 2024

I’ve done that. Let’s merge as-is if you’re good with it !

CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Max Inden <[email protected]>
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

🙏

@mxinden mxinden merged commit 4b78df1 into prometheus:master Jul 2, 2024
10 checks passed
@navaati navaati deleted the target_has_atomic branch July 2, 2024 08:48
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.

Some 32 bit platforms are not supported
2 participants