eunomia-bpf / bpftime

Userspace eBPF runtime for fast Uprobe & Syscall hook & Extensions with LLVM JIT
https://eunomia.dev/bpftime/
MIT License
699 stars 70 forks source link

ci: Fix CMakeLists.txt for llvm-jit to allow user define the llvm path #204

Closed Zheaoli closed 5 months ago

Zheaoli commented 6 months ago

Description

Fixes #187

In this PR, I make some changes following below:

  1. Use the LLVM managed by the system package manager
  2. Add LLVM_FIND_PATH to let people customize the LLVM path themself
  3. Add ENABLE_LLVM_SHARED for some special distributions which are make the LLVM all dynamic lib (like Arch Linux, FYI https://gitlab.archlinux.org/archlinux/packaging/packages/llvm/-/commit/3e452a15b68d156a9ef5608e8f6e1870d192d954)

Type of change

How Has This Been Tested?

Test Configuration:

Checklist

Zheaoli commented 6 months ago

@Officeyutong Hi, thanks for the comment, I can't quote your previous comment(would you mind to keep the discussion next time?)

After the discussion, I think it would be great keep only find_package(LLVM REQUIRED CONFIG) here. The developer could use the LLVM_ROOT or -DLLVM_ROOT to configure it. I think it would be better than hard code.

Meanwhile, the PR have following thing to do:

  1. Forbid LLVM-14
  2. Make CI Happy

WDYT?

Officeyutong commented 6 months ago

@Officeyutong Hi, thanks for the comment, I can't quote your previous comment(would you mind to keep the discussion next time?)

After the discussion, I think it would be great keep only find_package(LLVM REQUIRED CONFIG) here. The developer could use the LLVM_ROOT or -DLLVM_ROOT to configure it. I think it would be better than hard code.

Meanwhile, the PR have following thing to do:

  1. Forbid LLVM-14
  2. Make CI Happy

WDYT?

In fact I don't know which is better, may be you should consult @yunwei37

Officeyutong commented 6 months ago

@Officeyutong Hi, thanks for the comment, I can't quote your previous comment(would you mind to keep the discussion next time?) After the discussion, I think it would be great keep only find_package(LLVM REQUIRED CONFIG) here. The developer could use the LLVM_ROOT or -DLLVM_ROOT to configure it. I think it would be better than hard code. Meanwhile, the PR have following thing to do:

  1. Forbid LLVM-14
  2. Make CI Happy

WDYT?

In fact I don't know which is better, may be you should consult @yunwei37

Note that the following workflows should be updated:

yunwei37 commented 6 months ago

I think using find_package is ok.

Maybe we can update the bpftime build-and-test documents in eunomia.dev, to tell others the best way to compile it and set the cmake variables.

Thanks for your help!

yunwei37 commented 6 months ago

And also, is it better to delete the makefile in project root? There are too many options for the makefile to handle, maintaining the makefile and cmake together will also make user confused. What do you think?

If it's better, we can update the ci and documents in

https://eunomia.dev/bpftime/documents/build-and-test/

It can also help with #173

Zheaoli commented 6 months ago

I think it could be a split PR. I will simplify the build process and the doc

yunwei37 commented 6 months ago

Yes. It should be another issue and pr. Thanks a lot!

Zheaoli commented 5 months ago

Also fix #203

yunwei37 commented 5 months ago

LGTM, Thanks!