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

Improve package updates reporting and add more build deps to SDK #2615

Merged
merged 2 commits into from
Mar 5, 2025

Conversation

krnowak
Copy link
Member

@krnowak krnowak commented Jan 30, 2025

CI: http://jenkins.infra.kinvolk.io:8080/job/container/job/sdk/1956/cldsv/

This PR affects SDK only - it has some dependencies specified explicitly (for the package update automation) and it will now have all the build dependencies of the board packages, so the board-package-build job won't be building stuff for SDK anymore.

@krnowak krnowak added the main label Jan 30, 2025
@krnowak krnowak requested a review from a team February 4, 2025 11:22
@krnowak krnowak marked this pull request as ready for review February 4, 2025 11:22
@krnowak
Copy link
Member Author

krnowak commented Feb 4, 2025

CI passed.

Copy link

github-actions bot commented Feb 4, 2025

Build action triggered: https://github.com/flatcar/scripts/actions/runs/13675214714

@chewi
Copy link
Contributor

chewi commented Feb 12, 2025

I think there might be a better way, at least for including the build dependencies. In Portage 3.0.66, I changed --root-deps to install build dependencies to ROOT as well as / for all EAPIs. It previously installed them to ROOT instead of /, but this was never a good idea, so it was ignored for EAPI 7 and later.

@krnowak
Copy link
Member Author

krnowak commented Feb 13, 2025

I think there might be a better way, at least for including the build dependencies. In Portage 3.0.66, I changed --root-deps to install build dependencies to ROOT as well as / for all EAPIs. It previously installed them to ROOT instead of /, but this was never a good idea, so it was ignored for EAPI 7 and later.

The point here is that we want those build dependencies to be a part of the SDK, but it is not about where the build dependencies are installed (ROOT or /), but rather when they are being built. I want all the build dependencies of the board packages to be built and installed during the SDK build job. But there were still some build dependencies that were absent in SDK, so they were built during board packages job.

@chewi
Copy link
Contributor

chewi commented Feb 13, 2025

Ah, I see. --onlydeps --onlydeps-with-rdeps=n almost works, but it includes DEPEND, which is unhelpful in this case. Maybe I could add a --onlydeps-with-ddeps option or something? It's annoying that a lot of the existing Portage options lump DEPEND and BDEPEND together as build dependencies.

@krnowak
Copy link
Member Author

krnowak commented Feb 13, 2025

Ah, I see. --onlydeps --onlydeps-with-rdeps=n almost works, but it includes DEPEND, which is unhelpful in this case. Maybe I could add a --onlydeps-with-ddeps option or something?

I assume you meant --onlydeps-with-bdeps. Anyway. Do I understand it correctly that this flag would be something we could use in stage4 of SDK build, where we could replace emerge coreos-devel/sdk-depends with emerge --onlydeps-with-bdeps coreos-devel/board-packages; emerge coreos-devel/some-extra-pkgs-for-sdk? That'd be cool, actually.

It's annoying that a lot of the existing Portage options lump DEPEND and BDEPEND together as build dependencies.

Ah, well, that's the nature of the evolving software, isn't it? BDEPEND is relatively new.

@chewi
Copy link
Contributor

chewi commented Feb 13, 2025

I assume you meant --onlydeps-with-bdeps.

No, --onlydeps includes all the dependency types, then you filter out RDEPEND with --onlydeps-with-rdeps=n. Therefore, we would also filter out DEPEND with --onlydeps-with-ddeps=n. There's also --onlydeps-with-ideps for IDEPEND, which is filtered out by default when rdeps are, but we'd actually want to include those, so we'd also put =y for that.

Do I understand it correctly that this flag would be something we could use in stage4 of SDK build, where we could replace emerge coreos-devel/sdk-depends with emerge --onlydeps-with-bdeps coreos-devel/board-packages; emerge coreos-devel/some-extra-pkgs-for-sdk? That'd be cool, actually.

Yes. I've already coded it up and it works. Just gathering opinions on the option name.

Ah, well, that's the nature of the evolving software, isn't it? BDEPEND is relatively new.

New in 2018. 😛 I introduced it though, so I suppose it's my fault!

@krnowak
Copy link
Member Author

krnowak commented Feb 13, 2025

I assume you meant --onlydeps-with-bdeps.

No, --onlydeps includes all the dependency types, then you filter out RDEPEND with --onlydeps-with-rdeps=n. Therefore, we would also filter out DEPEND with --onlydeps-with-ddeps=n. There's also --onlydeps-with-ideps for IDEPEND, which is filtered out by default when rdeps are, but we'd actually want to include those, so we'd also put =y for that.

Got it. But wouldn't it be better to have --onlydeps=<CSV-of-dep-types>? So you could have dep types like rdeps, bdeps, ddeps, ideps and pdeps and you pass some variation of them to --only-deps. Possibly with a shortcut like --only-deps=all meaning --only-deps=rdeps,bdeps,ddeps,ideps,pdeps.

Do I understand it correctly that this flag would be something we could use in stage4 of SDK build, where we could replace emerge coreos-devel/sdk-depends with emerge --onlydeps-with-bdeps coreos-devel/board-packages; emerge coreos-devel/some-extra-pkgs-for-sdk? That'd be cool, actually.

Yes. I've already coded it up and it works. Just gathering opinions on the option name.

Cool. Do you want to add this feature to flatcar as a user-patch instead of a part of this PR? Or do we merge this PR and use your feature when it's merged into portage?

Ah, well, that's the nature of the evolving software, isn't it? BDEPEND is relatively new.

New in 2018. 😛 I introduced it though, so I suppose it's my fault!

That's why I said "relatively". :P

@chewi
Copy link
Contributor

chewi commented Feb 13, 2025

Got it. But wouldn't it be better to have --onlydeps=? So you could have dep types like rdeps, bdeps, ddeps, ideps and pdeps and you pass some variation of them to --only-deps. Possibly with a shortcut like --only-deps=all meaning --only-deps=rdeps,bdeps,ddeps,ideps,pdeps.

I thought that too, but we'd still need the old options for compatibility, and it would probably just be even more confusing than it already is.

Cool. Do you want to add this feature to flatcar as a user-patch instead of a part of this PR? Or do we merge this PR and use your feature when it's merged into portage?

The former. I've had one response that said ddeps is fine. Good enough.

@chewi
Copy link
Contributor

chewi commented Feb 13, 2025

I've checked the patch applies and pushed it. I'll leave you to plumb it in. I guess that should happen in catalyst_sdk.sh?

PORTAGE_CONFIGROOT="$ROOT" emerge --root="$ROOT" --sysroot="$ROOT" --onlydeps --onlydeps-with-rdeps=n --onlydeps-with-ddeps=n --onlydeps-with-ideps=y coreos-devel/board-packages

I haven't pushed it upstream yet, so please let me know if it works.

@krnowak
Copy link
Member Author

krnowak commented Feb 14, 2025

I've checked the patch applies and pushed it. I'll leave you to plumb it in. I guess that should happen in catalyst_sdk.sh?

PORTAGE_CONFIGROOT="$ROOT" emerge --root="$ROOT" --sysroot="$ROOT" --onlydeps --onlydeps-with-rdeps=n --onlydeps-with-ddeps=n --onlydeps-with-ideps=y coreos-devel/board-packages

I haven't pushed it upstream yet, so please let me know if it works.

Thanks, I'll try it out. I used a different command (run_merge, as provided by catalyst's chroot-functions.sh) though:

run_merge --onlydeps --onlydeps-with-rdeps=n --onlydeps-with-ddeps=n --onlydeps-with-ideps=y coreos-devel/board-packages

Let's see how it goes, I updated a link to the Jenkins build in the first message above.

@chewi
Copy link
Contributor

chewi commented Feb 14, 2025

Thanks, I'll try it out. I used a different command (run_merge, as provided by catalyst's chroot-functions.sh) though:

run_merge --onlydeps --onlydeps-with-rdeps=n --onlydeps-with-ddeps=n --onlydeps-with-ideps=y coreos-devel/board-packages

ROOT in that context refers to the SDK, not the board root. Although you're not actually installing any packages to the board root, you ideally want to use its profile etc. You therefore still need to PORTAGE_CONFIGROOT="$ROOT" run_merge --root="$ROOT" --sysroot="$ROOT" like build_target_toolchain() in catalyst_toolchains.sh does. I'd also do this at the end.

krnowak added 2 commits March 5, 2025 12:42
These dependencies are pulled into SDK at some point during the
multi-stage SDK build, but our package automation is not smart enough
to catch this. Help it by listing some packages explicitly.
Without those additions, these packages are being built into the SDK
during the board packages job.
@krnowak krnowak force-pushed the krnowak/fill-the-gaps branch from c53d81e to 309fb0e Compare March 5, 2025 11:42
@krnowak
Copy link
Member Author

krnowak commented Mar 5, 2025

Reverted to listing the packages explicitly - the portage patch did not work as expected.

@krnowak krnowak merged commit 810427f into main Mar 5, 2025
1 check was waiting
@krnowak krnowak deleted the krnowak/fill-the-gaps branch March 5, 2025 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants