-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
validate pure-rust build via CI #624
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Byron
force-pushed
the
ci-for-max-pure
branch
10 times, most recently
from
November 22, 2022 18:45
2e1cf42
to
35ef0d1
Compare
Byron
force-pushed
the
ci-for-max-pure
branch
from
November 22, 2022 18:50
35ef0d1
to
ed4deac
Compare
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 9, 2024
This makes the `pure-rust-build` CI job attempt to install gitoxide from `.` with implicit `max`, rather than explicit `max-pure`. This job, introduced in ed4deac (GitoxideLabs#624), is intended to identify unintended C toolchain dependencies. If, without the current breakage, it is (still) capable of doing this, then as temporarily modified here, it should fail. If it fails and the error messages clearly relate to the absence of components necessary to build/use C libraries, then it is probably (still) working. Assuming it fails as expected, the Debian version can then be increased, and then the changes in this commit reverted: if that fails, then succeeds, respectively, then the job's functionality is most likely preserved across the upgrade.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 9, 2024
This makes the `pure-rust-build` CI job attempt to install gitoxide from `.` with implicit `max`, rather than explicit `max-pure`. This job, introduced in ed4deac (GitoxideLabs#624), is intended to identify unintended C toolchain dependencies. If, without the current breakage, it is (still) capable of doing this, then as temporarily modified here, it should fail. If it fails and the error messages clearly relate to the absence of components necessary to build/use C libraries, then it is probably (still) working. Assuming it fails as expected, the Debian version can then be increased, and then the changes here (and in any follow-up testing adjustments) undone: if that fails, then succeeds, respectively, then the job's functionality is most likely preserved across the upgrade.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 9, 2024
This consists of several changes to verify that the `pure-rust-build` CI job (still) works to verify that building `max-pure` doesn't require tools it is not intended to require. The main goal of this testing is to allow each step to be undone after upgrading the Docker image to a newer version of Debian, while verifying that doing so doesn't break or otherwise weaken the test. See the individual commit messages below for details. * Temporarily break `pure-rust-build` to verify that it can fail This makes the `pure-rust-build` CI job attempt to install gitoxide from `.` with implicit `max`, rather than explicit `max-pure`. This job, introduced in ed4deac (GitoxideLabs#624), is intended to identify unintended C toolchain dependencies. If, without the current breakage, it is (still) capable of doing this, then as temporarily modified here, it should fail. If it fails and the error messages clearly relate to the absence of components necessary to build/use C libraries, then it is probably (still) working. Assuming it fails as expected, the Debian version can then be increased, and then the changes here (and in any follow-up testing adjustments) undone: if that fails, then succeeds, respectively, then the job's functionality is most likely preserved across the upgrade. * Temporarily add `cmake` to `pure-rust-build` job This needs to be undone, since `max-pure` definitely should not require `cmake`. The reason for this temporary change is to figure out how robust the test is: currently, it fails on `max` as it should, but reports the absence of `cmake` as the reason. For the test to be robust, its failure should be for a deeper reason, and preferably even implicate the `minimal` Rust profile. * Temporarily add `build-essential` to `pure-rust-build` job This still installs `gcc` and `libc-dev` because `build-essential` depends on them, but this als installs packages such as `g++` and `make`, the absence of which seems to trigger failure before the effects of installing the `minimal` toolchain do (if they do). As in the previous change, this will of course need to be undone. * Temporarily add `pkgconf` and `libssl-dev` to `pure-rust-build` job This too must of course be undone. Like the last couple temporary changes, this is to see if the significance of using the `minimal` Rust toolchain can be identified. Note: This causes the test to pass, even with the `minimal` Rust toolchain. That's okay. These effects can be compared to those of the reverse incremental changes to be made after increasing the Debian image version.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 9, 2024
This tests with various additional packages (tools and libraries) installed whose absence affected the `pure-rust-build` job, to establish the conditions under which it fails; then bumps the Debian 10 buster image to Debian 12 bookworm, becuase Debian 10 is past long-term support, and its Docker image is no longer being updated (see below for details); then redoes the original tests in reverse order to verify that each combination of packages had the same effect. This is several commit squashed together. (The shown reverted commit hashes may at some point become unavailable on the remote.) * Temporarily break `pure-rust-build` to verify that it can fail This makes the `pure-rust-build` CI job attempt to install gitoxide from `.` with implicit `max`, rather than explicit `max-pure`. This job, introduced in ed4deac (GitoxideLabs#624), is intended to identify unintended C toolchain dependencies. If, without the current breakage, it is (still) capable of doing this, then as temporarily modified here, it should fail. If it fails and the error messages clearly relate to the absence of components necessary to build/use C libraries, then it is probably (still) working. Assuming it fails as expected, the Debian version can then be increased, and then the changes here (and in any follow-up testing adjustments) undone: if that fails, then succeeds, respectively, then the job's functionality is most likely preserved across the upgrade. * Temporarily add `cmake` to `pure-rust-build` job This needs to be undone, since `max-pure` definitely should not require `cmake`. The reason for this temporary change is to figure out how robust the test is: currently, it fails on `max` as it should, but reports the absence of `cmake` as the reason. For the test to be robust, its failure should be for a deeper reason, and preferably even implicate the `minimal` Rust profile. * Temporarily add `build-essential` to `pure-rust-build` job This still installs `gcc` and `libc-dev` because `build-essential` depends on them, but this als installs packages such as `g++` and `make`, the absence of which seems to trigger failure before the effects of installing the `minimal` toolchain do (if they do). As in the previous change, this will of course need to be undone. * Temporarily add `pkgconf` and `libssl-dev` to `pure-rust-build` job This too must of course be undone. Like the last couple temporary changes, this is to see if the significance of using the `minimal` Rust toolchain can be identified. Note: This causes the test to pass, even with the `minimal` Rust toolchain. That's okay. These effects can be compared to those of the reverse incremental changes to be made after increasing the Debian image version. * Bump Docker image to Debian 12 for `pure-rust-build` It was previously Debian 10 buster. That version of Debian is past long term support (LTS), and although third-party Freexian ELTS updates are publicly available, the image on Docker Hub has not received any updates since the end of its LTS period. In contrast, the Debian 11 bullseye and Debian 12 bookworm images continue to be regularly updated, as of this writing. * Revert "Temporarily add `pkgconf` and `libssl-dev` to `pure-rust-build` job" This reverts commit 2ed0a37. * Revert "Temporarily add `build-essential` to `pure-rust-build` job" This reverts commit d9e1228. * Revert "Temporarily add `cmake` to `pure-rust-build` job" This reverts commit 26ebbe6. * Revert "Temporarily break `pure-rust-build` to verify that it can fail" This reverts commit b2e9289.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 9, 2024
This verifies the absence of utilities and libraries `max-pure` should not need, but that are needed for or, in the `libcurl*-dev` case, that may aid or modify the effect of, building `max`. When `pure-rust-build` was introduced in ed4deac (GitoxideLabs#624), the goal was to test that a C toolchain was not needed. Currently, we are installing a C toolchain, by installing `gcc` and `libc-dev`, so that the Rust toolchain will use the linker, which it may invoke through `cc`/`gcc`. Nonetheless, the test is effective, as verified in GitoxideLabs#1664, becuase it uses an environment free of several packages that `max-pure` would likely inadverently require for building, if it failed to be "pure".
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 9, 2024
This verifies the absence of utilities and libraries `max-pure` should not need, but that are needed for building `max`. When `pure-rust-build` was introduced in ed4deac (GitoxideLabs#624), the goal was to test that a C toolchain was not needed. Currently, we are installing a C toolchain, by installing `gcc` and `libc-dev`, so that the Rust toolchain will use the linker, which it may invoke through `cc`/`gcc`. Nonetheless, the test is effective, as verified in GitoxideLabs#1664, becuase it uses an environment free of several packages that `max-pure` would likely inadverently require for building, if it failed to be "pure".
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 9, 2024
This verifies the absence of utilities and libraries `max-pure` should not need, but that are needed for building `max`. When `pure-rust-build` was introduced in ed4deac (GitoxideLabs#624), the goal was to test that a C toolchain was not needed. Currently, we are installing a C toolchain, by installing `gcc` and `libc-dev`, so that the Rust toolchain will use the linker, which it may invoke through `cc`/`gcc`. Nonetheless, the test is effective, as verified in GitoxideLabs#1664, becuase it uses an environment free of several packages that `max-pure` would likely inadverently require for building, if it failed to be "pure". Utilities could, in principle, be installed as part of a package other than the package(s) that usually provide them. So a `$PATH` search is performed. However, `libssl-dev` is a library (and more libraries might be listed in the future), with no executable tool to do a `$PATH` search for. Furthermore, it may be possible for a utility to be installed, such that software in a Rust toolchain might find and use it, while not being in a `$PATH` directory. So this checks for known DEB packages as well as searching `$PATH`.
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 9, 2024
This verifies the absence of utilities and libraries `max-pure` should not need, but that are needed for building `max`. When `pure-rust-build` was introduced in ed4deac (GitoxideLabs#624), the goal was to test that a C toolchain was not needed. Currently, we are installing a C toolchain, by installing `gcc` and `libc-dev`, so that the Rust toolchain will use the linker, which it may invoke through `cc`/`gcc`. Nonetheless, the test is effective, as verified in GitoxideLabs#1664, becuase it uses an environment free of several packages that `max-pure` would likely inadverently require for building, if it failed to be "pure". Utilities could, in principle, be installed as part of a package other than the package(s) that usually provide them. So a `$PATH` search is performed. However, `libssl-dev` is a library (and more libraries might be listed in the future), with no executable tool to do a `$PATH` search for. Furthermore, it may be possible for a utility to be installed, such that software in a Rust toolchain might find and use it, while not being in a `$PATH` directory. So this checks for known DEB packages as well as searching `$PATH`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.