New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tools/7z: Allow building on alpine #12022
Conversation
@oliv3r did you actually manage to build a full image? For me it was a total fail |
Hey @Ansuel It was the only way, as otherwise I was getting For some reason, my build environment was borked, spanning a faked process, consuming 100% and causing my build to be single threaded, taking 30 minutes. On top of that, the resulting image failed to actually work, when starting init, I was getting 'attempted to kill init' panic. Which was super weird. The kernel booted fine in itself (which is where I do most of my work). Having tried many things (pulling newer openwrtorg docker containers, make distclean, manually rm -rf everything) something must have been kept somehere, causing this error. So I figured, Lets try building with an alpine container. Alpine docker container
And a new git worktree (ergo, all any any files will be gone). Without that 7z patch, 7zip refuses to build, for the same reasons that the alpine guys have added these patches. It might not work on non-musl distro's of course (debian) which would suck :( Also, since I'm on the realtec target, 7z is a hard dependency, as some images use it. |
Did you test that it still works fine on systems with glibc? |
It seemed to work; but probably not then :) though the changes don't see to invasive? But yeah, we'd have to look at that. But for whatever reason that I still don't understand (related to faked) I can't build glibc based systems reliably :S |
tools/7z/patches/7-zip-flags.patch
Outdated
@@ -0,0 +1,28 @@ | |||
diff -ru a/CPP/7zip/7zip_gcc.mak b/CPP/7zip/7zip_gcc.mak |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make tools/7z/{clean,refresh}
So it's failing for me too; however, only because it cannot download the actual source code; which doesn't seem to relate to my changes? fail log
I used the following debian container Docker debian
|
@oliv3r By any chance, are you on a corporate network with a MITM proxy/firewall such as a Fortigate? If so, you'll likely find that Docker containers need your company's private CA certs injected into them or they'll do exactly this because they're choking on the Fortigate (or similar MITM monitoring proxy). I have yet to find any way to automagically have Docker containers use the host CA cert store. |
Huh what? nah, this is a plain old ISP fiber connection, with a very open ISP (Can even run mail servers on port 25) without any firewall. Besides, I've been building openwrt and deps for months now, and it worked a few days ago on my alpine container. I'll try again though :) edit: So it's much more logical then that, I didn't have |
@Ansuel how was it exploding for you? With the previously linked debian container (its updated with ca-certificates) :p it works 'just fine'?
@neheb so it took me a while to figure out that this 'quiltifies' the patch :p First it was just erroring out, with some quilt error, so I went through the quilt dance, just to find out it's not perfectly quilt formatted. Quilt needs to be improved a bit and be less 'special snowflake' and more like 'this is what diff generates and be happy with some of it' :p (though I understand some of the reasons ...). @hauke as per log, builds just fine, seems to run just fine as well. |
@Ansuel @hauke I just did a clean rebulid of the debian container, including a clean worktree, so basically, starting from 100% scratch. The tree I used is my own, which includes all the realtek changes, but rebased ontop of master. Works fine, no problems (except for those pesky |
|
||
// ret2 = | ||
- pthread_attr_setaffinity_np(&attr, sizeof(*cpuSet), cpuSet); | ||
+ //pthread_attr_setaffinity_np(&attr, sizeof(*cpuSet), cpuSet); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will deactivate the pthread_attr_setaffinity_np
feature also on glibc builds.
I do not think this call is important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but that's then still a performance aspect only? @hauke Do you happen to know a nice define we can throw in there? Something that would always be set on glibc?
Thinking a bit more about it, the impact should be tiny. It only affects the host. The CI is unlikely to use it, as those are restricted often on resources anyway. The scripts don't specify multiple thread usage, and we're talking about image generation, which is 4 - 16MiB files. So IF multi-threaded operation is requested, we miss out the option to set the affinity (which I'm not even sure how we'd do it from the openwrt build system.
Rectifying it can be easily done with a define (once someone has shared one :D but I'm not gonna try to find out, as the impact is likely negligible for our workloads I believe.
When using alpine as host, things start to fail. Lets pull in the upstream alpine patches to make things work. This should not affect other hosts. Note, that Alpine has the '_GNU_SOURCE' define in the APKBUILD file, but here we add this flag to the needed fix flags patch, which does similar things too. Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
When using alpine as host, things start to fail. Lets pull in the upstream alpine patches to make things work. This should not affect other hosts.
Note, that Alpine has the '_GNU_SOURCE' define in the APKBUILD file, but here we add this flag to the needed fix flags patch, which does similar things too.