-
Notifications
You must be signed in to change notification settings - Fork 296
ci: Move linux builds to the container #7563
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
Conversation
1c6883c to
886ba93
Compare
844ca33 to
6a599a7
Compare
|
I looked into these errors from I tried using the target with Lines 717 to 725 in 5c454ec
And indeed I could reproduce the issue using it: |
|
We attempted to compare the What we found was that While it could on our CI host: And the error was: Which indicates that the |
|
The we found out that LTO actually works correctly when Which means that the Nix wrapping of GCC is causing issues with LTO plugin. When unwrapped GCC is used it fails on missing Glibc instead: |
|
If we set Which show that the original |
|
I have managed to make it work by passing Which confirms the issue is with wrapped GCC and LTO support when loading symbols from |
d4abbc1 to
6f93b3f
Compare
|
Posts with same issue: What I've found is this line in the The LTO does the run time optimizations, but we are using it only in the tests and not when building the node itself. I will do a test where I'll run the tests with and without the LTO enabled to see if it impacts significantly to the performance. If not, I think it's safe to disable it. |
d0cc741 to
6f93b3f
Compare
|
In the normal build without nix, we are having an increase of 5 min because of disabling LTO in the tests step: The Nix build failed even though in the shell hook I've set the NIM_PARAMS to disable LTO, but maybe the outer quotes got stripped. I've rerun the build with setting the variable in the test step like this: it passed the step, now I'm waiting for the total time. |
6f93b3f to
f002c34
Compare
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.
Why is this a separate Jenkinsfile from ci/Jenkinsfile?
Because we are using a docker container and since the same jenkinsfile is used for macos builds this would fail. |
e675e2c to
bf34d0f
Compare
In general, Nimbus ships an LTO build, so it'd be better to test with it too. |
fb5c58b to
df55374
Compare
798e3a1 to
9a11c8d
Compare
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.
It's really really really dumb how we can't reuse the same Jenkinsfile because of the dockerfile block being in a static agent block parsed early on.
But what we could do is define stages in a common.groovy file to not repeat the same thing twice. But that can be tried separately.
9a11c8d to
3b8de86
Compare
3b8de86 to
98136b8
Compare
We are in the process of moving pipelines across organization to linux docker containers. For the nimbus pipelines we have only two relevant ones:
The problem we noticed was when running tests within the nix shell in the container because the GCC doesn't come with LTO support and thus we had to disable it for tests. The new successfull builds don't show any degradation in the pipeline performance in terms of duration.
Referenced issue: