Replace build wrapper with native meson targets #1722
Conversation
|
Build failed. ✔️ unit-test SUCCESS in 2m 02s |
PeakKS
force-pushed
the
redo-build-system
branch
from
88bdd17 to
a4c63b1
Compare
|
Build failed. ❌ unit-test FAILURE in 1m 34s |
|
Build failed. ❌ unit-test FAILURE in 1m 34s |
|
Not sure how CI is failing, logs seem to show the binary getting compiled then not existing for completions? |
debarshiray
left a comment
debarshiray
left a comment
There was a problem hiding this comment.
Thanks for working on this, @PeakKS ! I don't have time right now to take a very close look, but I didn't want to completely ignore your significant contribution. So some fly-by comments:
First, I think what you are suggesting is a good idea. The less shell scripts we use, the better it is! :)
Second, do you think it will be possible to have tests to check if the toolbox binary was correctly built? You have already seen what src/go-build-wrapper was doing, and you will see the background in the Git history of that file. The toolbox binary needs to be linked in a very specific way for different Toolbx containers to work across different host OSes.
If we have tests to check the important attributes of the binary, then it becomes a lot easier to make changes to the build, and review those changes, etc.. It's one of the big reasons why we never revamped src/go-build-wrapper before. Manually checking everything felt too nerve-wracking and stressful. :)
debarshiray
left a comment
debarshiray
left a comment
There was a problem hiding this comment.
Could you please use your full name and a real email address in your Git authorship information?
The Signed-off-by trailer has no meaning without a real full name and email address.
I could have made an exception for a one line change because it would be legally insignificant. However, that's not the case with this pull request, because you have done a substantial amount of great work. :)
We need a real full name and email address for tracking copyright holders in case a licensing issue crops up. Note how projects as diverse as GCC, GnuPG, Linux and Podman don't allow anonymous or pseudonymous contributions.
Signed-off-by: Blake Batson <[email protected]>
Signed-off-by: Blake Batson <[email protected]>
Signed-off-by: Blake Batson <[email protected]>
Signed-off-by: Blake Batson <[email protected]>
Signed-off-by: Blake Batson <[email protected]>
PeakKS
force-pushed
the
redo-build-system
branch
from
1b925a9 to
fd565e8
Compare
No problem, didn't even realize I used my username lol. |
|
Build failed. ❌ unit-test FAILURE in 1m 37s |
2 participants
Tentative fix for #1706
Replaces the go build wrapper and the completions wrapper with just doing the build right in meson. Essentially I am just tricking go into building as a static library, then linking that with itself. This prevents the weird go build cache issue I ran into in #1706.
Since
-z nowcan not be used for this application, I am forcing-z lazyin the linker args. It seems like the reason this only popped up on gentoo is because we default to-z nowfor security reasons (see : https://packages.gentoo.org/useflags/default-znow).Getting rid of some of the boilerplate is just a nice side benefit :)