Skip to content
This repository was archived by the owner on Jul 11, 2023. It is now read-only.

Upgrade to LLVM 10 #66

Merged
merged 1 commit into from
Jul 27, 2020
Merged

Upgrade to LLVM 10 #66

merged 1 commit into from
Jul 27, 2020

Conversation

aquarhead
Copy link

@aquarhead aquarhead commented Jul 23, 2020

(Forgot to mention the reasoning, Rust 1.45 is using LLVM 10 now which is the merge commit from this PR)

The code change should be trivial.

I think we might want to clean up the README a bit, for Ubuntu dependencies:

  • Don't think we need curl wget or gnupg2 at all?
  • Don't think we need ca-certificates-java either?
  • It seems libelf is enough (I just realized I didn't have libelf-dev - though installing it doesn't solve the load issue I have, and it certainly didn't affect cargo bpf build)
  • zlib1g is needed (forgot about the error message but it's the first error I hit when running cargo bpf build)
  • linux-headers need to pick an explicit version, otherwise apt (apt-get too) complains E: Package 'linux-headers' has no installation candidate

It might also be related to Ubuntu releases, I'm running on 20.04 (Desktop).

@alessandrod
Copy link
Contributor

Thanks! I'll take a look soon, I'll rebase my related changes on top of this.

@rsdy
Copy link
Collaborator

rsdy commented Jul 23, 2020

This is great, thank you!

I'm wondering if it's worth making this change backwards compatible, or should we enforce a hard dependency on LLVM10. It looks like Bionic and newer Ubuntu versions have a proper llvm-10 in the repo, and Arch already packages LLVM9 as llvm-9, so if we adjust the CI, there should be little impact in moving forward, and we don't want to support older Fedora anyway.

@alessandrod
Copy link
Contributor

I've tested this PR together with #67 and things do work again with stable rust.

Keeping backwards compatibility isn't worth it since we operate on LLVM bitcode which isn't stable, so for things to work we must use the same LLVM version used by rustc which at the moment is 10.

The only thing that's missing to merge this is changing the CI images to install llvm-10 instead of llvm-9.

@aquarhead
Copy link
Author

Can confirm adding #67 works :)

changing the CI images to install llvm-10 instead of llvm-9

While you're at it, can you also take a look at other dependencies? I'll follow up in this PR to further cleanup the dependencies list if you let me know the minimum set required.

@rsdy
Copy link
Collaborator

rsdy commented Jul 27, 2020

We have some CI issues at the moment I'll need to take a look at. Thanks both for the contribution, I'll merge #67 along with this.

@rsdy rsdy merged commit cec92fa into foniod:master Jul 27, 2020
@aquarhead aquarhead deleted the llvm-10 branch July 27, 2020 15:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants