Skip to content

Commit

Permalink
Rework Toolchain model and drop relative file path overrides
Browse files Browse the repository at this point in the history
Relative path overrides permit a freshly downloaded source tree to
execute arbitrary code on any rustup command that executes a binary from
the configured toolchain, and its a reasonable tradeoff for us to remove
this feature. Absolute path overrides are kept intact - these were added
to support users of large monorepo tool systems, and can be kept with
reasonable safety.

Introduce an interior model for toolchain names which allows the
elimination of conditional code for custom vs named vs path based
toolchains. Fixes #3130.

Finishes the separation of Toolchain vs DistributableToolchain vs
CustomToolchain started some time ago.

Moves some test support into the src tree in order to keep symbols
private and adds a 'test' feature required for self-testing, ensuring
that that code doesn't affect production builds.

Changes the CI debug builds to use the new test feature and also otel,
so that otel doesn't bitrot.
  • Loading branch information
rbtcollins committed May 15, 2023
1 parent 4b29b79 commit 2c9297d
Show file tree
Hide file tree
Showing 73 changed files with 3,663 additions and 2,449 deletions.
46 changes: 46 additions & 0 deletions .github/workflows/all-features.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: AllFeatures (PR)

# This is an additional workflow to test building with all feature combinations.
# Unlike our main workflows, this doesn't self-test the rustup install scripts,
# nor run on the rust docker images. This permits a smaller workflow without the
# templating and so on.

on:
pull_request:
branches:
- "*"
- renovate/*

jobs:
build:
name: Build
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
# Might add more targets in future.
target:
- x86_64-unknown-linux-gnu
steps:
- name: Clone repo
uses: actions/checkout@v3
- name: Install rustup stable
run: rustup toolchain install stable --profile minimal
- name: Install Protoc
uses: arduino/setup-protoc@v1
with:
version: "3.x"
- name: Set environment variables appropriately for the build
run: |
echo "$HOME/.cargo/bin" >> $GITHUB_PATH
echo "TARGET=${{ matrix.target }}" >> $GITHUB_ENV
- name: Cache Cargo
uses: Swatinem/rust-cache@v2
- name: Install cargo-all-features
run: cargo install cargo-all-features --git https://github.com/rbtcollins/cargo-all-features.git
- name: Ensure we have our goal target installed
run: |
rustup target install "$TARGET"
- name: Build every combination
run: |
cargo check-all-features --root-only
11 changes: 8 additions & 3 deletions .github/workflows/centos-fmt-clippy-on-all.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ jobs:
strategy:
fail-fast: false
steps:
- uses: actions/checkout@v3
- name: Clone repo
uses: actions/checkout@v3
with:
# v2 defaults to a shallow checkout, but we need at least to the previous tag
fetch-depth: 0
Expand Down Expand Up @@ -71,6 +72,10 @@ jobs:
run: |
rustup component add rustfmt
rustup component add clippy
- name: Install Protoc
uses: arduino/setup-protoc@v1
with:
version: '3.x'
- name: Run the centos check within the docker image
run: |
docker run \
Expand All @@ -91,6 +96,6 @@ jobs:
cargo fmt --all --check
- name: Run cargo check and clippy
run: |
cargo check --all --all-targets
cargo check --all --all-targets --features test
git ls-files -- '*.rs' | xargs touch
cargo clippy --all --all-targets
cargo clippy --all --all-targets --features test
4 changes: 4 additions & 0 deletions .github/workflows/linux-builds-on-master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ jobs:
echo "DOCKER=$DOCKER" >> $GITHUB_ENV
- name: Fetch the docker
run: bash ci/fetch-rust-docker.bash "${TARGET}"
- name: Install Protoc
uses: arduino/setup-protoc@v1
with:
version: '3.x'
- name: Maybe build a docker from there
run: |
if [ -f "ci/docker/$DOCKER/Dockerfile" ]; then
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/linux-builds-on-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ jobs:
echo "DOCKER=$DOCKER" >> $GITHUB_ENV
- name: Fetch the docker
run: bash ci/fetch-rust-docker.bash "${TARGET}"
- name: Install Protoc
uses: arduino/setup-protoc@v1
with:
version: '3.x'
- name: Maybe build a docker from there
run: |
if [ -f "ci/docker/$DOCKER/Dockerfile" ]; then
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/linux-builds-on-stable.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ jobs:
echo "DOCKER=$DOCKER" >> $GITHUB_ENV
- name: Fetch the docker
run: bash ci/fetch-rust-docker.bash "${TARGET}"
- name: Install Protoc
uses: arduino/setup-protoc@v1
with:
version: '3.x'
- name: Maybe build a docker from there
run: |
if [ -f "ci/docker/$DOCKER/Dockerfile" ]; then
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/windows-builds-on-master.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ jobs:
TARGET: ${{ matrix.target }}
# os-specific code leads to lints escaping if we only run this in one target
run: |
cargo check --all --all-targets
cargo check --all --all-targets --features test
git ls-files -- '*.rs' | xargs touch
cargo clippy --workspace --all-targets
cargo clippy --workspace --all-targets --features test
- name: Upload the built artifact
if: matrix.mode == 'release'
uses: actions/upload-artifact@v3
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/windows-builds-on-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ jobs:
TARGET: ${{ matrix.target }}
# os-specific code leads to lints escaping if we only run this in one target
run: |
cargo check --all --all-targets
cargo check --all --all-targets --features test
git ls-files -- '*.rs' | xargs touch
cargo clippy --workspace --all-targets
cargo clippy --workspace --all-targets --features test
- name: Upload the built artifact
if: matrix.mode == 'release'
uses: actions/upload-artifact@v3
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/windows-builds-on-stable.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ jobs:
TARGET: ${{ matrix.target }}
# os-specific code leads to lints escaping if we only run this in one target
run: |
cargo check --all --all-targets
cargo check --all --all-targets --features test
git ls-files -- '*.rs' | xargs touch
cargo clippy --workspace --all-targets
cargo clippy --workspace --all-targets --features test
- name: Upload the built artifact
if: matrix.mode == 'release'
uses: actions/upload-artifact@v3
Expand Down
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

1. Fork it!
2. Create your feature branch: `git checkout -b my-new-feature`
3. Test it: `cargo test`
3. Test it: `cargo test --features=test`
4. Lint it: `cargo +beta clippy --all --all-targets -- -D warnings`
> We use `cargo clippy` to ensure high-quality code and to enforce a set of best practices for Rust programming. However, not all lints provided by `cargo clippy` are relevant or applicable to our project.
> We may choose to ignore some lints if they are unstable, experimental, or specific to our project.
Expand Down Expand Up @@ -312,7 +312,7 @@ And [look in Jaeger for a trace](http://localhost:16686/search?service=rustup).

The custom macro `rustup_macros::test` adds a prelude and suffix to each test to
ensure that there is a tracing context setup, that the test function is a span,
and that the spans from the test are flushed. Build with features=otel to
and that the spans from the test are flushed. Build with features=otel,test to
use this feature.

### Adding instrumentation
Expand Down
113 changes: 105 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 2c9297d

Please sign in to comment.