anakryiko / retsnoop

Investigate kernel error call stacks
BSD 2-Clause "Simplified" License
186 stars 32 forks source link

Add release workflow for shipping binaries and combined sources #42

Closed qmonnet closed 1 year ago

qmonnet commented 1 year ago

Same as for veristat. Minor differences include:

Note that compiling the binary on arm64 takes much longer in the case of retsnoop, apparently because of the Rust tool: it took 20 minutes for my test run (vs. ~2min for amd64).

Test release available here.

anakryiko commented 1 year ago

I'd like to keep all the naming and composition of source archive exactly the same as for the manually created release, so that ArchLinux and Fedora scripts work with no modification. I can do that after applying, but if you have few minutes to see what needs changing, I'd really appreciate it. Thanks!

Strange about 20 minute build for arm64. It's no big deal, releases don't happen all that often, so waiting for 20 minutes is ok.

qmonnet commented 1 year ago

Right, I forgot that you already had something in place for this repo. I can certaily update the names accordingly.

Note that the structure of the archive won't be 100% similar. Is this an issue? In particular, it seems that with git-archive-all.sh, the submodules are independently compressed, and included into the toplevel archive (as ./bpftool.zip, ./bpftool.libbpf.zip, ./libbpf.zip), such that you need an additional step to uncompress them before you can compile. This is something we could replicate I suppose, or we could check out and call git-archive-all.sh directly from the workflow.

The alternative that I have in the current workflow is to check out all submodules recursively and then make a single global archive without the .git directories, but I don't know how that would work with the packaging scripts you mention.

anakryiko commented 1 year ago

Right, I forgot that you already had something in place for this repo. I can certaily update the names accordingly.

Note that the structure of the archive won't be 100% similar. Is this an issue? In particular, it seems that with git-archive-all.sh, the submodules are independently compressed, and included into the toplevel archive (as ./bpftool.zip, ./bpftool.libbpf.zip, ./libbpf.zip), such that you need an additional step to uncompress them before you can compile. This is something we could replicate I suppose, or we could check out and call git-archive-all.sh directly from the workflow.

The alternative that I have in the current workflow is to check out all submodules recursively and then make a single global archive without the .git directories, but I don't know how that would work with the packaging scripts you mention.

yeah, I'm not sure. Let's ask packagers if that's a big deal or it should be easy to adjust their scripts. @michel-slm and @Decave any concerns? Would the new format work for you guys?

Byte-Lab commented 1 year ago

It shouldn't be a problem for Arch. We can add whatever logic we need to the build() directive.

qmonnet commented 1 year ago

(OK sounds good, I updated the current PR with the names in use in the meantime [except for the binaries that keep an _amd64/_arm64 suffix for distinction], and added the .zip archive for sources. New test release available here if anyone needs to compare the assets with the existing ones.)

anakryiko commented 1 year ago

Great, thanks! I'll merge this now, but will make sure to check with @michel-slm before cutting a new release that it's good with him as well.

@qmonnet one thing worth noting and perhaps improving (in general) is to take into account .gitattributes file in libbpf. See https://github.com/libbpf/libbpf/blob/master/.gitattributes, it makes git archive ignore assets/ subfolder with images and reduces the size of archives by about 1.5-2MBs. Is it possible to somehow utilize that with your approach as well?

qmonnet commented 1 year ago

Is it possible to somehow utilize that with your approach as well?

Good question.

Sounds like we should come up with something to package sources with submodules and without ignored files, and make it an action to share between bpftool, retsnoop, and veristat, probably.

qmonnet commented 1 year ago

On a quick search, https://github.com/Kentzo/git-archive-all could maybe help

anakryiko commented 1 year ago

On a quick search, https://github.com/Kentzo/git-archive-all could maybe help

it's probably similar to what I've been doing manually with https://github.com/anakryiko/retsnoop/blob/master/scripts/archive-srcs-full.sh ? So yeah, it seems like using git archive through some shared github action seems like the best solution.

qmonnet commented 1 year ago

it's probably similar to [...]

Yes, https://github.com/Kentzo/git-archive-all is close to https://github.com/fabacab/git-archive-all.sh. I like that it packs the project in a single archive, instead of doing separate archives for each submodule (which requires more steps to uncompress all). It works with export-ignore from .gitattributes too, but the one drawback I found is a difference of behaviour when using rules with directory names only (for example, assets/ export-ignore instead of assets/** export-ignore), which should be easy to address if necessary.

So I started to experiment with it, and created an action to run this script in an easy way. I've generated a release on my retsnoop fork, and everything in the asset archives seems in order. I can finalise the action (by adding a README, tagging a version) and submit PRs to retsnoop and veristat, if it looks good to you.

anakryiko commented 1 year ago

so https://github.com/libbpf/libbpf/pull/692 should fix the problem, is that right?

And thanks again for taking care of this!

qmonnet commented 1 year ago

so https://github.com/libbpf/libbpf/pull/692 should fix the problem, is that right?

I'll test (tomorrow) to be sure, but yeah, I expect so.