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

CI: Use llvm-tools-preview toolchain instead of external llvm-tools toolchain #1903

Open
3 of 5 tasks
briansmith opened this issue Jan 13, 2024 · 2 comments
Open
3 of 5 tasks

Comments

@briansmith
Copy link
Owner

briansmith commented Jan 13, 2024

  • In mk/{install-build-tools.sh, should not do install_packages llvm-$llvm_version. Instead we should install the tools using rustup +$toolchain toolchain component add llvm-tools-preview before the coverage runs. This would require us to parse "+$toolchain" out of the command line, much like we parse "--target=$target". We'd need to update ci.yml accordingly to pass +${{ matrix.rust_channel }} on the command line. In ml/cargo.sh, instead of using external LLVM tools for code code coverage analysis, we should use the llvm-tools-preview tools installed in the previous step.
  • In mk/check-symbol-prefixes.sh, we should similarly switch to using the llvm-tools-preview llvm-nm, and change ci.yml accordingly.
  • We should change the conditional logic in ci.yml so that mk/check-symbol-prefixes.sh is run for apple targets, where it is currently being skipped.
  • We should change the conditional logic in ci.yml so that mk/check-symbol-prefixes.sh is run for Windows targets. (We can force the use of bash as the shell for Windows). This may require tweaking some of the logic.
  • We should change the conditional logic in ci.yml so that mk/check-symbol-prefixes.sh is run for all remaining targets, if at all possible.

Each checklist item above should be done as a separate PR, ideally.

@briansmith
Copy link
Owner Author

PR #1908 attempts to resolve the mk/check-symbol-prefixes.sh parts.

@briansmith
Copy link
Owner Author

  • mk/{install-build-tools.sh, should not do install_packages llvm-$llvm_version

It still needs to, but only when we are in $use_clang mode.

Instead we should install the tools using rustup +$toolchain toolchain component add llvm-tools-preview before the coverage runs. This would require us to parse "+$toolchain" out of the command line, much like we parse "--target=$target". We'd need to update ci.yml accordingly to pass +${{ matrix.rust_channel }} on the command line.

Done in PR #1980.

In mk/check-symbol-prefixes.sh, we should similarly switch to using the llvm-tools-preview llvm-nm, and change ci.yml accordingly.
We should change the conditional logic in ci.yml so that mk/check-symbol-prefixes.sh is run for apple targets, where it is currently being skipped.

Done in PR #1908.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant