search

Xilinx-CNS / tcpdirect

AMD TCPDirect ultra low latency kernel bypass TCP and UDP implementation for AMD Solarflare network adapters, to be used with corresponding versions of Onload®️ at https://github.com/Xilinx-CNS/onload. The stable branch is currently `v8_1`.
22 stars 16 forks source link

ON-15664: Generate SRPM with sources and not pre-built binaries #29

Closed ivatet-amd closed 3 months ago

ivatet-amd commented 3 months ago

Put TCPDirect source files instead of pre-built binaries into the SRPM package generated by the zf_make_official_srpm script.

It is quite a crude PR because, for instance, it does not list the tarball files explicitly and can catch temporary text editor files into the tarball, but there are more series to come to address it.

TODO:

krishd-amd commented 3 months ago

Putting the bash scripts into shellcheck seems to show some linting problems. Would you mind addressing them?

Probably should add a github action to default lint scripts.

ivatet-amd commented 3 months ago

Thanks @krishd-amd. I've shellchecked the entire zf_make_official_srpm and snippets from other files. The warnings mainly were about double-quoting variables ${var} -> "${var}", which I definitely agree with. Please ping me if your shellcheck spots more problems!

ivatet-amd commented 3 months ago

Thank you for the prompt reviews! I've either addressed the comments as agreed or captured follow-up suggestions as tasks in ON-15695.

Overall, the SPEC file still requires a lot of care, but its improvements depend on some heavy lifting, such as refactoring Makefile and scripts like zf_mkdist.

ivatet-amd commented 3 months ago

Thanks for the review @pcolledg-amd. The git-archive approach is certainly an improvement over tar, but I'm not applying it here because it wouldn't work with the CI as-is: it doesn't stash the .git subdirectory.

The latest PR addresses your inline comments.

There is one pending problem with the binary RPM workflow in this PR, which I will fix shortly. (It won't substantially change the PR.)

ivatet-amd commented 3 months ago

The latest push addresses the tar/symlink transformation issue and adds the S suffix to the expression that prepends the tcpdirect-${version} prefix to all files in the tarball. Without it, transformation applies to the symlink destination and messes it up.

Before (a broken symlink):

$ ls -l tcpdirect-foo/src/tests/zf_apps/ | grep Makefile-top
lrwxrwxrwx 1 iteterev sflr    39 Mar 27 10:25 Makefile-top.inc -> tcpdirect-foo/../../../Makefile-top.inc

After (👍):

$ ls -l tcpdirect-foo/src/tests/zf_apps/ | grep Makefile-top
lrwxrwxrwx 1 iteterev sflr    25 Mar 27 10:25 Makefile-top.inc -> ../../../Makefile-top.inc

https://www.gnu.org/software/tar/manual/html_node/transform.html

The latest personal pipeline is amber/green, so planning to merge it shortly. Thanks for the reviews!