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

Add a new CI job to check that smithy-rs compiles on 32bit Linux-based targets #1812

Merged
merged 46 commits into from
Oct 10, 2022
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
46 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
b171398
Add a new CI job to check that smithy-rs compiles on 32bit Linux-base…
LukeMathWalker Oct 5, 2022
28cfd52
No trailing commas pretty please.
LukeMathWalker Oct 5, 2022
c39f51f
Point cross at the manifest explicitly.
LukeMathWalker Oct 5, 2022
d374749
Skip crates with a Python dependency. Exercise all features.
LukeMathWalker Oct 5, 2022
0a35e57
Install required dependencies.
LukeMathWalker Oct 5, 2022
04da581
Do not fail fast, we want to see the result on all platforms.
LukeMathWalker Oct 5, 2022
889cf30
Set paths for both commands.
LukeMathWalker Oct 5, 2022
68a8ff7
Openssl must be installed inside the cross Docker container, not on t…
LukeMathWalker Oct 5, 2022
6489be4
Fix connector setup if `rustls` feature is not enabled.
LukeMathWalker Oct 5, 2022
faa7c0c
Restrict feature set on powerpc.
LukeMathWalker Oct 5, 2022
656566b
Pass openssl env variables to the cross Docker container
LukeMathWalker Oct 5, 2022
763c184
Split in two commands.
LukeMathWalker Oct 5, 2022
2aeafc9
Enable debug level logs.
LukeMathWalker Oct 5, 2022
b79c5bc
Remove openssl feature (temporarily).
LukeMathWalker Oct 5, 2022
f121f3f
Raise verbosity.
LukeMathWalker Oct 5, 2022
19e5d28
Trigger CI
LukeMathWalker Oct 5, 2022
27c9b0d
`native-tls`, here we go again.
LukeMathWalker Oct 5, 2022
8c10c19
Clean up.
LukeMathWalker Oct 5, 2022
c6e78b9
Merge branch 'main' into 32-bits
LukeMathWalker Oct 5, 2022
95c8b70
Merge branch '32-bits' into exotic-ci
LukeMathWalker Oct 5, 2022
f904443
Trigger CI
LukeMathWalker Oct 5, 2022
9d116a1
Trigger CI
LukeMathWalker Oct 5, 2022
5755c38
Add pkg-config.
LukeMathWalker Oct 5, 2022
7169ba7
Change include path to include arch
LukeMathWalker Oct 5, 2022
a56e151
Trigger CI
LukeMathWalker Oct 5, 2022
b151d4d
Allow workflow_dispatch on ci-pr to enable triggering this CI workflo…
LukeMathWalker Oct 5, 2022
c1321f6
Trigger CI? Why are you doing this to me GitHub?
LukeMathWalker Oct 5, 2022
d9b37b3
Trigger CI? Are you alive GitHub?
LukeMathWalker Oct 5, 2022
7f697cc
Fix env variables for openssl
LukeMathWalker Oct 6, 2022
1d1685b
Use features only for rust-runtime crates.
LukeMathWalker Oct 6, 2022
58d35cc
Check all feature combinations for aws-smithy-client
LukeMathWalker Oct 6, 2022
cc9d2c8
Dry-up env variables.
LukeMathWalker Oct 6, 2022
9b2ee06
Merge branch 'main' into exotic-ci
LukeMathWalker Oct 6, 2022
a1c7b35
Merge branch 'main' into exotic-ci
LukeMathWalker Oct 6, 2022
ef7002a
A rogue `echo` was missing
LukeMathWalker Oct 6, 2022
01e98a0
Feature-gate doc tests based on the features they require.
LukeMathWalker Oct 6, 2022
58a2822
Put .github folder under shared ownership.
LukeMathWalker Oct 6, 2022
a3bb7c7
Fix docs.
LukeMathWalker Oct 6, 2022
6e1a354
Fix feature selection for doctest.
LukeMathWalker Oct 6, 2022
3796190
We are using methods that are only available if rustls is enabled - a…
LukeMathWalker Oct 6, 2022
dd857e8
Merge branch 'main' into exotic-ci
LukeMathWalker Oct 7, 2022
d47fba6
Merge branch 'main' into exotic-ci
LukeMathWalker Oct 10, 2022
7881449
Remove workflow dispatch trigger.
LukeMathWalker Oct 10, 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
1 change: 1 addition & 0 deletions .github/workflows/ci-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

name: CI
on:
workflow_dispatch:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this for?

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 gives you the possibility to trigger the workflow from GitHub's UI.
See ci-main, which already has it:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Neat!

Copy link
Collaborator

Choose a reason for hiding this comment

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

But why would you need this for ci-pr.yml? It runs every time a PR is updated.

And if it is run via workflow dispatch, how does it get values needed by the PR bot:

      issue_number: ${{ github.event.number }}
      base_revision: ${{ github.event.pull_request.base.sha }}
      head_revision: ${{ github.event.pull_request.head.sha }}

pull_request:

# Allow one instance of this workflow per pull request, and cancel older runs when new changes are pushed
Expand Down
68 changes: 68 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,73 @@ jobs:
popd &>/dev/null
done

# We make sure that Smithy-rs can be compiled on platforms that are not natively supported by GitHub actions.
# We do not run tests on those platforms (yet) because it'd require a more complicated setup involving architecture
# emulation via QEMU, likely to cause a significant degradation on CI completion time.
test-exotic-platform-support:
name: Exotic platform support
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
include:
- target: i686-unknown-linux-gnu
features: --all-features
aws_excludes: ''
# We only test `native-tls` here because `rustls` depends on `ring` which in turn does not support powerpc
# as a target platform (see https://github.com/briansmith/ring/issues/389)
# We also exclude all first-party crates that have a non-optional dependency on `ring`.
- target: powerpc-unknown-linux-gnu
features: --features native-tls
aws_excludes: --exclude aws-inlineable --exclude aws-sigv4 --exclude aws-sig-auth
steps:
- name: Checkout
uses: actions/checkout@v1
# Pinned to the commit hash of v1.3.0
- uses: Swatinem/rust-cache@842ef286fff290e445b90b4002cc9807c3669641
with:
sharedKey: ${{ runner.os }}-${{ env.rust_version }}-${{ github.job }}-${{ matrix.target }}
target-dir: ./target
- uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.rust_version }}
components: ${{ env.rust_toolchain_components }}
profile: minimal
override: true
target: ${{ matrix.target }}
- name: Configure cross
shell: bash
run: |
cat > Cross.toml << EOF
[build]
pre-build = ["dpkg --add-architecture i386", "apt-get update && apt-get install --assume-yes pkg-config:i386 libssl-dev:i386"]
[build.env]
passthrough = [
"OPENSSL_LIB_DIR",
"OPENSSL_INCLUDE_DIR",
]
EOF
- name: Build rust-runtime crates
uses: actions-rs/cargo@v1
env:
CROSS_CONFIG: Cross.toml
OPENSSL_LIB_DIR: /usr/lib/i386-linux-gnu/
OPENSSL_INCLUDE_DIR: /usr/include/i386-linux-gnu/openssl/
with:
use-cross: true
command: build
args: --target ${{ matrix.target }} --manifest-path "rust-runtime/Cargo.toml" --exclude aws-smithy-http-server-python --workspace ${{ matrix.features }}
- name: Build AWS rust-runtime crates
uses: actions-rs/cargo@v1
env:
CROSS_CONFIG: Cross.toml
OPENSSL_LIB_DIR: /usr/lib/i386-linux-gnu/
OPENSSL_INCLUDE_DIR: /usr/include/i386-linux-gnu/openssl/
with:
use-cross: true
command: build
args: --target ${{ matrix.target }} --manifest-path "aws/rust-runtime/Cargo.toml" ${{ matrix.aws_excludes }} --workspace ${{ matrix.features }}

# This job is split out from the rest since it is not required to pass for merge
check-sdk-examples:
name: Check SDK Examples
Expand All @@ -166,6 +233,7 @@ jobs:
- test-codegen
- test-sdk
- test-rust-windows
- test-exotic-platform-support
# Run this job even if its dependency jobs fail
if: always()
runs-on: ubuntu-latest
Expand Down
16 changes: 16 additions & 0 deletions CHANGELOG.next.toml
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"

[[smithy-rs]]
message = """
Mark `operation` and `operation_handler` modules as private in the generated server crate.
Expand Down
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-client/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl<M, R> Builder<(), M, R> {
let with_https = |b: Builder<_, M, R>| b.rustls_connector(connector_settings);
// If we are compiling this function & rustls is not enabled, then native-tls MUST be enabled
#[cfg(not(feature = "rustls"))]
let with_https = |b: Builder<_, M, R>| b.native_tls_connector();
let with_https = |b: Builder<_, M, R>| b.native_tls_connector(connector_settings);
LukeMathWalker marked this conversation as resolved.
Show resolved Hide resolved

with_https(self)
}
Expand Down
6 changes: 3 additions & 3 deletions rust-runtime/aws-smithy-client/src/never.rs
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
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",
))]
mod tests {
use super::*;

Expand Down
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