Skip to content
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

ci: Test more targets, move the testing logic to a script #28

Closed
wants to merge 1 commit into from

Conversation

vadorovsky
Copy link
Contributor

@vadorovsky vadorovsky commented Nov 12, 2024

Cover all possible targets. The ones which fail to build, are disabled with an appropriate comment explaining the error.

To make the tests easier to reproduce locally, provide the container_test.sh script and move the most of logic there.

@vadorovsky
Copy link
Contributor Author

vadorovsky commented Nov 12, 2024

The last red build on main took 40 minutes, which means that's the time for the toolchain to be ready. Workflows can run for 3-6 hours, so I will try to build the whole system here. If that takes too long, I will fall back to installing zstd (or some other package).

@thesamesam
Copy link
Member

I would like to cover a lot more targets so maybe we could only bother with the full @system test for selected ones?

- riscv64-unknown-linux-gnu
- riscv64-unknown-linux-musl
- sparc-unknown-linux-gnu
- sparc-unknown-linux-musl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no sparc* port for musl (or musl port for sparc*), unfortunately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, removed

@vadorovsky
Copy link
Contributor Author

vadorovsky commented Nov 13, 2024

Now that we have some jobs failing, I realized that the CI jobs are hard to debug, because the outputs of emerge are redirected to log files. Would you be open to having an option to print the failure logs to terminal (like --log-errors or --log-failures)? I could add it in a separate commit.

I will also make the whole script fail realy in the next push.

@thesamesam
Copy link
Member

Now that we have some jobs failing, I realized that the CI jobs are hard to debug, because the outputs of emerge are redirected to log files. Would you be open to having an option to print the failure logs to terminal (like --log-errors or --log-failures)? I could add it in a separate commit.

I will also make the whole script fail realy in the next push.

I suppose, not sure if I love the name though. --capture-errors maybe.

- riscv64-unknown-linux-musl
- sparc-unknown-linux-gnu
- sparc64-unknown-linux-gnu
- sparc64-unknown-linux-musl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to go too.

@vadorovsky
Copy link
Contributor Author

Now that we have some jobs failing, I realized that the CI jobs are hard to debug, because the outputs of emerge are redirected to log files. Would you be open to having an option to print the failure logs to terminal (like --log-errors or --log-failures)? I could add it in a separate commit.
I will also make the whole script fail realy in the next push.

I suppose, not sure if I love the name though. --capture-errors maybe.

Nevermind, there is --show-fail-log already.

@thesamesam
Copy link
Member

I had no idea!

@vadorovsky vadorovsky changed the title ci: Build the base system after bootstraping the toolchain ci: Build the base system after bootstraping the toolchain, add more architectures Nov 13, 2024
@vadorovsky vadorovsky force-pushed the fix-ci-for-real branch 6 times, most recently from 29dd526 to 0e45474 Compare November 19, 2024 15:47
@vadorovsky vadorovsky force-pushed the fix-ci-for-real branch 3 times, most recently from b0963f6 to a804317 Compare November 23, 2024 07:30
@vadorovsky vadorovsky changed the title ci: Build the base system after bootstraping the toolchain, add more architectures ci: Test more targets, move the testing logic to a script Nov 23, 2024
@vadorovsky vadorovsky force-pushed the fix-ci-for-real branch 5 times, most recently from 5bfd73d to 6b8830e Compare November 24, 2024 12:36
@vadorovsky vadorovsky force-pushed the fix-ci-for-real branch 4 times, most recently from de10492 to 4d70cf1 Compare November 25, 2024 07:36
@vadorovsky
Copy link
Contributor Author

vadorovsky commented Nov 25, 2024

@thesamesam Are you by any chance able to restart single jobs? These two failed just because of flakyness of emerge --sync:

https://github.com/gentoo/crossdev/actions/runs/12005351093/job/33461826634?pr=28
https://github.com/gentoo/crossdev/actions/runs/12005351093/job/33461827068?pr=28

These targets worked before.

If not, I will force push, but I'm wondering if we can avoid that. I'm usually able to restart jobs in repositories I own.

In general, I think we're almost there. Once everything is green, I think we are good to go.

@thesamesam
Copy link
Member

Unfortunately, I can't (the permissions most people have are very limited on the gentoo github repos). But maybe it's a sign of a real bug, actually (or a real risk of flakiness): let's change it to emerge-webrsync instead so we get the nightly snapshots and we're guaranteed all jobs used the same tree state?

Cover all possible targets. The ones which fail to build, are disabled
with an appropriate comment explaining the error.

To make the tests easier to reproduce locally, provide the
`container_test.sh` script and move the most of logic there.

Signed-off-by: Michal Rostecki <[email protected]>
@thesamesam
Copy link
Member

thesamesam commented Nov 25, 2024

Looks like we're good to go? Thanks a lot for your work. I'll wait until you switch to ready before merging.

I have always been a bit uneasy about crossdev changes because of the lack of such automated testing.

@vadorovsky vadorovsky marked this pull request as ready for review November 25, 2024 15:51
@vadorovsky
Copy link
Contributor Author

I marked it as ready. Thank you for your review! Looking forward to test my LLVM-related changes with it.

BTW, would you be open for using Github Actions in the main gentoo repo as well? I know that there is Tinderbox and https://qa-reports.gentoo.org/. But from my understanding, Tinderbox tests mostly already merged changes and the QA reports on pull requests are mostly syntax linters? I think a Github Action which tries to ebuild the given package in containers would prevent issues like https://bugs.gentoo.org/943129.

@thesamesam
Copy link
Member

I think that's a more controversial question that would require broader discussion. It would be nice for PRs at least to help catch errors like that though, I agree. There have been a lot of different efforts in the past to have tinderboxing-setup-as-regular-CI like Zorry is doing at the moment with buildbot, but relying on github actions would kind of be advantageous here anyway for untrusted/arbitrary PRs anyway...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants