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

Update Cargo.toml to use LLVM-12 #146

Merged
merged 2 commits into from
Aug 23, 2021

Conversation

rennergade
Copy link
Contributor

Minor update to use newer LLVM version

@CLAassistant
Copy link

CLAassistant commented Aug 18, 2021

CLA assistant check
All committers have signed the CLA.

@woodruffw
Copy link
Member

woodruffw commented Aug 19, 2021

The CI also probably needs to be updated to install LLVM 12 instead (it probably passes coincidentally at the moment, because of a default installation). Would you mind doing that as well?

@rennergade
Copy link
Contributor Author

Sure. I updated the versions. Is there a way to test this implicitly?

@smoelius
Copy link
Collaborator

Sure. I updated the versions. Is there a way to test this implicitly?

I think the policy we should adopt is: the LLVM version to which siderophile is pinned should be the same as that of the latest stable rust.

$ rustc --version --verbose
rustc 1.54.0 (a178d0322 2021-07-26)
binary: rustc
commit-hash: a178d0322ce20e33eac124758e837cbd80a6f633
commit-date: 2021-07-26
host: x86_64-apple-darwin
release: 1.54.0
LLVM version: 12.0.1
              ^^^^^^

siderophile insists on using the default compiler. siderophile should check that the default compiler uses the LLVM version it expects. And, as a consequence, CI should fail when there is a mismatch.

I will open a separate issue for this, and we can continue the discussion there.

@smoelius
Copy link
Collaborator

smoelius commented Aug 23, 2021

We will also have to update the README (here), but I can do that after we merge.

@woodruffw I am going to merge this and close #145 if this looks good to you.

@smoelius smoelius merged commit 0bf859f into trailofbits:master Aug 23, 2021
@smoelius
Copy link
Collaborator

Thanks a lot, @rennergade.

@smoelius smoelius mentioned this pull request Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants