alegrey91 / harpoon

🔍 Trace syscalls from user-space functions, by using eBPF
Apache License 2.0
88 stars 3 forks source link

golangci: try to set up golangci #43

Open ccoVeille opened 1 month ago

ccoVeille commented 1 month ago

The ebpf libraries are not helping at all.

ccoVeille commented 1 month ago

@alegrey91 Please review

I suggest you to consider merging it, and fixing the issues one by one

I set up the test as non-blocking, I could separate the golangci-lint in a dedidated jobs, just tell me

if you want to launch golangci-lint locally please use the following command

golangci-lint run internal/analyzer internal/archiver internal/elfreader internal/embeddable internal/executor internal/metadata harpoon/internal/privileged internal/seccomputils internal/w
riter
alegrey91 commented 1 month ago

This solution doesn't work well. We can't remember to add any new packages to the list of the lint. Maybe is better to add a non-linting directive on top of the ebpf library functions.

alegrey91 commented 1 month ago

Additionally, I would keep this job in a dedicated pipeline for linting.

ccoVeille commented 1 month ago

This solution doesn't work well. We can't remember to add any new packages to the list of the lint. Maybe is better to add a non-linting directive on top of the ebpf library functions.

I know it's a pain.

I wanted to proof it was possible.

We will have to look at GitHub action on how to pass the list of package.

There is unfortunately no way to ask a linter to ignore a file. It reads everything, and any typecheck issue are breaking everything.

It's what is happening with this file depending on the external lib.

There might be a way to refactor, by using distinct modules in the code, but it would need a huge rewrite.

ccoVeille commented 1 month ago

Additionally, I would keep this job in a dedicated pipeline for linting.

Noted. I will split it when I have a chance to look at a way to set a variable by calling a shell command or script

alegrey91 commented 1 month ago

I know it's a pain.

I wanted to proof it was possible.

We will have to look at GitHub action on how to pass the list of package.

There is unfortunately no way to ask a linter to ignore a file. It reads everything, and any typecheck issue are breaking everything.

It's what is happening with this file depending on the external lib.

There might be a way to refactor, by using distinct modules in the code, but it would need a huge rewrite

What about something like this? https://stackoverflow.com/questions/65985835/how-skip-file-in-golangci-lint

ccoVeille commented 1 month ago

I don't expect this to work, but they are among the idea I plan to try

ccoVeille commented 1 month ago

Here is it what we could use to store the list of packages by using the grep command you had

https://github.com/thomast1906/thomasthorntoncloud-examples/pull/92

Another solution could be to split the code that can be tested in a package in internal and leave the one that depends on the external lib in another.

Something like:

cmd internal/ok/ internal/lib/

This way you only test and run golangci-lint-config-examples on internal/ok

alegrey91 commented 1 month ago

what if we use something like instead?

skip-dirs:
    - internal/ebpf
    - ebpf/

This would be easier

ccoVeille commented 1 month ago

It cannot work unfortunately. Longtime debated in golangci-lint ecosystem. golangci-lint has to parse everything, but if it finds code that cannot be compiled. It would fail. So using skipdir cannot work.

It's worth trying, but I'm pretty sure it won't work the way we need.

Note: I'm busy with family this weekend, I cannot try it.