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

Drop LLVM 5 codegen tests #1215

Closed
fbs opened this issue Mar 17, 2020 · 3 comments · Fixed by #1262
Closed

Drop LLVM 5 codegen tests #1215

fbs opened this issue Mar 17, 2020 · 3 comments · Fixed by #1262
Labels
RFC Request for comment

Comments

@fbs
Copy link
Member

fbs commented Mar 17, 2020

We currently have codegen tests for LLVM 5 - 9 (10 in a PR). The code we generate is basically the same for each LLVM version (the only LLVM_VERSION ifdefs are the mem* calls) so there is not much to test between versions there. That we're able to automatically rewrite most of the tests confirms that.
The main advantage of having codegen tests for each version is probably that it makes development easy, you can use the LLVM your OS has instead of having everyone use the same version.

Especially the LLVM5 tests mainly feel like a maintenance burden. The automatic rewriting fails in a lot of places (17) and the CI job has a massive test filter too.
Are there objections to dropping LLVM 5 from the codegen tests? We still have the runtime and other tests to verify that everything is working. This might cause some issues for people that still use LLVM5 as their codegen tests will fail (unless we code the exception into the test runner) but I don't think there are many (any) LLVM5 users left. Even ubunt 16.04 has LLVM8

Or we could drop LLVM5 support completely, removing it completely (and use the glibc static(ish) builds instead).

@danobi
Copy link
Member

danobi commented Mar 17, 2020

+1 on dropping

@danobi danobi added the RFC Request for comment label Mar 17, 2020
@mmisono
Copy link
Collaborator

mmisono commented Mar 30, 2020

LLVM 10 test is merged: #1190
+1 on dropping

@acj
Copy link
Contributor

acj commented Mar 31, 2020

Another +1 for dropping

Or we could drop LLVM5 support completely, removing it completely (and use the glibc static(ish) builds instead).

fwiw, #1063 removes the static LLVM 5 builds in favor of LLVM 9, still on Alpine as before.

fbs added a commit to fbs/bpftrace that referenced this issue Apr 14, 2020
The LLVM 5 builds are flaky and a pain to maintain. Linking against musl
leads to all kinds of weird crashes which limits usefulness of the
static binaries. As we now have a working static build based on ubuntu
18.04 we can get rid of this and make development a bit easier.

All the major LTS distros seem to have LLVM6+ support:

- Ubuntu 16.04 has LLVM8
- Debian jessie/8 (almost EOL) has LLVM6
- Centos 8 has LLVM8
- Suse has 6+ available

Fixes bpftrace#1215
This was referenced Apr 14, 2020
@fbs fbs closed this as completed in #1262 Apr 14, 2020
fbs added a commit that referenced this issue Apr 14, 2020
The LLVM 5 builds are flaky and a pain to maintain. Linking against musl
leads to all kinds of weird crashes which limits usefulness of the
static binaries. As we now have a working static build based on ubuntu
18.04 we can get rid of this and make development a bit easier.

All the major LTS distros seem to have LLVM6+ support:

- Ubuntu 16.04 has LLVM8
- Debian jessie/8 (almost EOL) has LLVM6
- Centos 8 has LLVM8
- Suse has 6+ available

Fixes #1215
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants