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

github/workflows: Added the platform suffix to the linuxkit cache. #4579

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

yash-zededa
Copy link
Collaborator

@yash-zededa yash-zededa commented Feb 3, 2025

Packages built for different platforms are stored in a shared cache based on the architecture.

This PR introduces platform-specific cache suffixes for EVE, ensuring it retrieves the correct cache. This prevents build failures caused by architecture-dependent dependencies.

Packages built for different platforms are stored in the same cache
based on the arch.

This PR will creates cache based on the platform suffix for eve to
refer the it from the cache.

Signed-off-by: yash-zededa <[email protected]>
@yash-zededa yash-zededa marked this pull request as ready for review February 3, 2025 14:14
Copy link
Member

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

LGTM, good catch, since we are building variants with platform we should do that

@yash-zededa yash-zededa requested a review from rene February 3, 2025 14:22
@deitch
Copy link
Contributor

deitch commented Feb 3, 2025

Given the structure this is correct. Is this the right structure though? Doesn't this mean that we rebuild all of the packages except for those that have platform variants? I know, it's prior to this PR - and this PR fixes a clear bug - but let's take advantage to have the conversation.

One matrix run builds arm64-generic, all packages. The next builds arm64-nvidia-jp6, which builds generic for all packages except those that have a variant. Most of the time, those already will be published, so it won't matter, but if a few have changed, that will make it take a lot longer.

@uncleDecart
Copy link
Member

I mean using GitHub cache also will reduce pull rate from docker, which could potentially hit the infamous pull rate limit, won't it?

@deitch
Copy link
Contributor

deitch commented Feb 3, 2025

Yes but that wasn't what I was concerned about. Or were you responding to something else?

@uncleDecart
Copy link
Member

uncleDecart commented Feb 3, 2025

Most of the time, those already will be published, so it won't matter, but if a few have changed, that will make it take a lot longer.

I was responding to this :)

One matrix run builds arm64-generic, all packages. The next builds arm64-nvidia-jp6, which builds generic for all packages except those that have a variant

I got what you're saying, so we won't skip generic packages when we build Nvidia-jp6? And if I build arm64-generic and arm64-nvidia-jp6 on the same machine, will Makefile be smart enough to skip already build generic packages when building Nvidia-jp6?

@deitch
Copy link
Contributor

deitch commented Feb 3, 2025

Is eve a sponsored OSS package on docker hub? If it is, those pulls will not matter.

if I build arm64-generic and arm64-nvidia-jp6 on the same machine, will Makefile be smart enough to skip already build generic packages when building Nvidia-jp6?

As far as I recall, when you build one platform, it builds (or pulls) everything, trying platform specific before falling back to generic.

As for same machine, no such thing. Matrix runs on GHA get their own VMs.

This makes me wonder if this would not be easier with the other approach we discussed. Where make pkgs (or some variant) builds all platforms. Then we would have one build set and once cache per architecture.

@uncleDecart
Copy link
Member

As for same machine, no such thing. Matrix runs on GHA get their own VMs.

True, but, we can technically run not a matrix, but multiple steps, way worse code, wouldn't like going there, but it's an option, trying to understand, where exactly we do things and how :)

@rene
Copy link
Contributor

rene commented Feb 3, 2025

This discussion is really relevant, but we need this fix anyways in the current workflow, so I've created the following issue: #4580 to continue the discussion so we can merge this PR.

@rene rene merged commit 784df17 into lf-edge:master Feb 3, 2025
21 checks passed
@yash-zededa yash-zededa deleted the add-platform-suffix-cache branch February 3, 2025 17:23
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.

4 participants