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.

Sign up for GitHub

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

Support 32bits targets #1811

Merged
merged 5 commits into from
Oct 6, 2022
Merged

Support 32bits targets #1811

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d260446
Replace AtomicU64 with AtomicUsize to prevent compilation issues on 3…
LukeMathWalker Oct 5, 2022
2ca4dc5
Make sure that Rust tests compile on MacOS.
LukeMathWalker Oct 5, 2022
56b0562
Add CHANGELOG next entry.
LukeMathWalker Oct 5, 2022
c6e78b9
Merge branch 'main' into 32-bits
LukeMathWalker Oct 5, 2022
cba1f0e
Merge branch 'main' into 32-bits
LukeMathWalker Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions CHANGELOG.next.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@
# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
# author = "rcoh"

[[smithy-rs]]
message = """
Replace all usages of `AtomicU64` with `AtomicUsize` to support 32bit targets.
"""
references = ["smithy-rs#1811"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "server"}
author = "LukeMathWalker"

[[smithy-rs]]
message = """
Replace all usages of `AtomicU64` with `AtomicUsize` to support 32bit targets.
"""
references = ["smithy-rs#1811"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client"}
author = "LukeMathWalker"
Comment on lines +14 to +28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's breaking only for the client - let me know if there are better ways to convey it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this conveys the problem 👍🏼


[[smithy-rs]]
message = """
Mark `operation` and `operation_handler` modules as private in the generated server crate.
Expand Down
6 changes: 3 additions & 3 deletions rust-runtime/aws-smithy-client/src/never.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use http::Uri;
use aws_smithy_async::future::never::Never;

use std::marker::PhantomData;
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;

use std::task::{Context, Poll};
Expand All @@ -27,7 +27,7 @@ use tower::BoxError;
#[derive(Debug)]
pub struct NeverService<Req, Resp, Err> {
_resp: PhantomData<(Req, Resp, Err)>,
invocations: Arc<AtomicU64>,
invocations: Arc<AtomicUsize>,
}

impl<Req, Resp, Err> Clone for NeverService<Req, Resp, Err> {
Expand Down Expand Up @@ -55,7 +55,7 @@ impl<Req, Resp, Err> NeverService<Req, Resp, Err> {
}

/// Returns the number of invocations made to this service
pub fn num_calls(&self) -> u64 {
pub fn num_calls(&self) -> usize {
self.invocations.load(Ordering::SeqCst)
}
}
Expand Down
8 changes: 8 additions & 0 deletions rust-runtime/aws-smithy-http-server-python/src/socket.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@ impl PySocket {
}

#[cfg(test)]
// `is_listener` on `Socket` is only available on certain platforms.
// In particular, this fails to compile on MacOS.
#[cfg(any(
target_os = "android",
target_os = "freebsd",
target_os = "fuchsia",
target_os = "linux",
))]
Comment on lines +76 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can still run the socket_can_be_cloned test below in MacOS, it looks like the is_listener call in it is unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on the intention behind the test - was the idea to explicitly verify that the cloned socket is in a listening state after cloning? @crisidev

Copy link
Contributor

Choose a reason for hiding this comment

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

That was the intention.

mod tests {
use super::*;

Expand Down
4 changes: 2 additions & 2 deletions rust-runtime/aws-smithy-http-server/examples/pokemon-service/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use std::{
collections::HashMap,
convert::TryInto,
sync::{atomic::AtomicU64, Arc},
sync::{atomic::AtomicUsize, Arc},
};

use async_stream::stream;
Expand Down Expand Up @@ -108,7 +108,7 @@ struct PokemonTranslations {
#[derive(Debug)]
pub struct State {
pokemons_translations: HashMap<String, PokemonTranslations>,
call_count: AtomicU64,
call_count: AtomicUsize,
}

impl Default for State {
Expand Down