nix: add building oid to the flake #502
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
JakeHillion
force-pushed
the
nix2
branch
11 times, most recently
from
65cb3b4 to
5a1a134
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #502 +/- ##
=======================================
Coverage 60.78% 60.79%
=======================================
Files 126 126
Lines 12491 12491
Branches 2014 2014
=======================================
+ Hits 7593 7594 +1
+ Misses 3941 3940 -1
Partials 957 957 ☔ View full report in Codecov by Sentry. |
JakeHillion
force-pushed
the
nix2
branch
12 times, most recently
from
83ab72a to
0883355
Compare
JakeHillion
force-pushed
the
nix2
branch
2 times, most recently
from
c7682c4 to
d5a7301
Compare
ajor
approved these changes
Aug 16, 2024
| --output-junit results.xml | ||
| - name: upload results | ||
| uses: actions/upload-artifact@v4 | ||
| if: success() || failure() |
There was a problem hiding this comment.
What other possibility is there?
There was a problem hiding this comment.
I think that's all of them. The implicit default is if: success() meaning the test analysis wouldn't get to run if any tests failed.
| @@ -170,7 +171,10 @@ def pull_base_toml() -> typing.Dict: | |||
|
|
|||
| # Now, we need to replace any placeholders that might be present in the base toml file with the real verisons. | |||
| user = getpass.getuser() | |||
| pwd = str(repo_path.resolve()) | |||
| if "IN_NIX_SHELL" in os.environ and "src" in os.environ: | |||
| pwd = os.environ['src'] | |||
There was a problem hiding this comment.
Nix. I was surprised it isn't called NIX_SRC_DIR or something. I'm using this one so the placeholders get replaced with /nix/store store paths, rather than paths into your repo. This makes the output stable and also if the config file is ever a Nix output it'll have the correct dependencies.
OI's build is challenging and has often been a problem for the Open Source community. It requires an extremely specific set of dependencies that are very hard to achieve on most systems. There are frequent breakages, like when updating to CentOS Stream 9, or when trying to update the CI's clang from clang-12 to clang-15 - OI requires the clang libraries to be version 15 but can't be compiled with it on the CI! This changes provides a mostly working build environment with `nix`. This environment is pinned to a specific nixpkgs revision using `flake.lock`, and only updates when we explicitly tell it to. Summary of changes: - Update CMakeLists.txt required version to 3.24. This allows specifying `FIND_PACKAGE_ARGS` in `FetchContent`, meaning we can use system packages. This is available on most up to date distros (3.30.2 is current). - Extends `flake.nix` to be able to build OI. Adds instructions for building and developing OI using `nix`. - Partially runs the tests in GitHub Actions. A huge amount must be excluded because of incompatibilites between the clangStdenv compiler and drgn. We have similar, though fewer, issues when building with the clang-12/libstdcxx mix on the Ubuntu 22.04 CircleCI, though this is at least reproducible. - Updates CircleCI to build CMake from source as we don't have a newer image available. Also add some newly found dependencies (not sure how it was working without them before). Test plan: This change requires less testing than previous build related changes because it deprecates most of the build types. - The internal BUCK build is unaffected. No special testing. - The semi-internal CMake build is gone. Use Nix. - The Nix build for clang-15 and some tests are continuously tested in GitHub actions. - Tested the set of Nix commands in the README. All work except the one that points to GitHub as this must be merged first. - The existing CircleCI runs on Ubuntu 20.04 are maintained. - Unable to test the new `test-report.yml` as it must be merged due to the permissions it needs. Will follow up with testing after this is merged. See: https://github.com/dorny/test-reporter?tab=readme-ov-file#recommended-setup-for-public-repositories The list of exclusions for GitHub Actions/nix testing is currently very long, I think 29% of the tests. This should be stable and reproducible though, and likely needs deep changes to OI to fix. That's why fixes are excluded from this PR. It's all to do with the forked drgn not being able to parse clang's newer DWARF output, and can't be fixed by rolling back as we required a relatively new libcxx.
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.
OI's build is challenging and has often been a problem for the Open Source
community. It requires an extremely specific set of dependencies that are very
hard to achieve on most systems. There are frequent breakages, like when
updating to CentOS Stream 9, or when trying to update the CI's clang from
clang-12 to clang-15 - OI requires the clang libraries to be version 15 but
can't be compiled with it on the CI!
This changes provides a mostly working build environment with
nix. Thisenvironment is pinned to a specific nixpkgs revision using
flake.lock, andonly updates when we explicitly tell it to.
Summary of changes:
FIND_PACKAGE_ARGSinFetchContent, meaning we can use system packages.This is available on most up to date distros (3.30.2 is current).
flake.nixto be able to build OI. Adds instructions for buildingand developing OI using
nix.because of incompatibilites between the clangStdenv compiler and drgn. We
have similar, though fewer, issues when building with the clang-12/libstdcxx
mix on the Ubuntu 22.04 CircleCI, though this is at least reproducible.
available. Also add some newly found dependencies (not sure how it was
working without them before).
Test plan:
This change requires less testing than previous build related changes because
it deprecates most of the build types.
actions.
points to GitHub as this must be merged first.
test-report.ymlas it must be merged due to thepermissions it needs. Will follow up with testing after this is merged. See:
https://github.com/dorny/test-reporter?tab=readme-ov-file#recommended-setup-for-public-repositories
The list of exclusions for GitHub Actions/nix testing is currently very long, I
think 29% of the tests. This should be stable and reproducible though, and
likely needs deep changes to OI to fix. That's why fixes are excluded from this
PR. It's all to do with the forked drgn not being able to parse clang's newer
DWARF output, and can't be fixed by rolling back as we required a relatively
new libcxx.