Skip to content

Conversation

@deitch
Copy link
Contributor

@deitch deitch commented Jun 19, 2023

glibc.yaml has a runtime dependency on wolfi-baselayout. This created a loop:

  1. wolfi-baselayout depends runtime on glibc-local-posix
  2. which is a subpackage of glibc
  3. glibc, in turn, depends runtime on wolfi-baselayout.

At @kaniini 's suggestion, removing the glibc runtime (not buildtime) dependency on wolfi-basealayout.

To be honest, I am not sure the right way to test this. If it were buildtime, it would be easy: either the package builds correctly or it does not. Do we have anything in tests that catches this?

@deitch deitch requested a review from a team as a code owner June 19, 2023 17:16
@deitch deitch requested review from imjasonh and joshrwolf June 19, 2023 17:16
@deitch
Copy link
Contributor Author

deitch commented Jun 19, 2023

Not enabling auto-merge until someone can comment on the question above.

@dlorenc
Copy link
Member

dlorenc commented Jun 19, 2023

I think we'd need to check all images that include glibc and make sure that they'd still include Wolfi base layout.

@deitch
Copy link
Contributor Author

deitch commented Jun 19, 2023

That sounds like a good job for an automated process.

Actually, let me ask. Does glibc actually need wolfi-baselayout? The way I read Ariadne's comment was, "no, it doesn't". Are you saying it does, so we push it downstream? Or are you saying it does not, but other downstream packages used this as a crutch?

@deitch deitch force-pushed the glibc-remove-wolfi-baselayout branch 2 times, most recently from fb662ce to ad49e23 Compare June 19, 2023 17:40
@kaniini
Copy link
Contributor

kaniini commented Jun 20, 2023

The package itself does not need it, but the underlying images do.

@deitch
Copy link
Contributor Author

deitch commented Jun 20, 2023

underlying images do

Do you mean the images that consume glibc?

  • If it is "images in chainguard-images/images that use glibc, likely also need wolfi-baselayout," I would think that is the responsibility of the downstream image to declare, "I need wolfi-baselayout"; why would glibc provide the unrelated wolfi-baselayout?
  • If it is "images that use glibc cannot use it without wolfi-baselayout, i.e. glibc is useless without it", then I would think that glibc itself really needs it, and it should be declared here (which creates a runtime dependency loop, bringing us back where we started).

@deitch deitch force-pushed the glibc-remove-wolfi-baselayout branch from ad49e23 to 00bd72a Compare June 20, 2023 07:39
@kaniini
Copy link
Contributor

kaniini commented Jun 20, 2023

Right, to clarify, the glibc package is usable without wolfi-baselayout, but the wolfi-baselayout package sets up some symlinks and other things that make sure glibc and other packages are laid out the desired way in the images we build using wolfi.

One option might be to split out the “required” path setup and have glibc provide that as a subpackage which it depends on, which wolfi-baselayout then pulls in as a dependency.

Or we could just say “you always need a baselayout apk.”

Part of the reason why glibc has an runtime dep on wolfi-baselayout is to ensure the solver picks it before glibc though…

@deitch
Copy link
Contributor Author

deitch commented Jun 20, 2023

Or we could just say “you always need a baselayout apk.”

Instinctively, I lean towards this, as you probably can tell from my earlier comments. It is clean and makes sense.

Except for the two points you raised:

the wolfi-baselayout package sets up some symlinks and other things that make sure glibc and other packages are laid out the desired way in the images we build using wolfi.

and

ensure the solver picks it before glibc though

Which are valid reasons.

Maybe an alternate approach?

wolfi-baselayout is the "base layout" and only depends on:

  • ca-certificates-bundle, which has zero other dependencies
  • glibc-locale-posix, which creates the cycle here

If we always want wolfi-baselayout to be first, then why does it depend on glibc-locale-posix? Can we remove it, or make it a standalone package? If we do that, we get everything we need:

  • wolfi-baselayout does not depend on glibc
  • glibc depends on wolfi-baselayout, including its various symlinks and directories and such

Thoughts?

@kaniini
Copy link
Contributor

kaniini commented Jun 20, 2023

We could remove the posix dependency from the baselayout, instead making it a runtime dep of glibc. What do you think?

@imjasonh
Copy link
Member

I think we'd need to check all images that include glibc and make sure that they'd still include Wolfi base layout.

All images require wolfi-baselayout. It's not linted currently but could be relatively easily. The migration to TF will make it even harder to mess up, since it's a provider-wide package defined once.

@deitch
Copy link
Contributor Author

deitch commented Jun 21, 2023

We could remove the posix dependency from the baselayout, instead making it a runtime dep of glibc. What do you think?

We wouldn't need to. If we remove it from wolfi-baselayout, then it already is a subpackage of glibc.

The question is, why is it a dep of wolfi-baselayout? Is it needed or assumed to be there?

Conversely, what if we make it part of wolfi-baselayout? glibc already depends on wolfi-baselayout. Is that the easiest route?

Instead of:

glibc -> wolfi-baselayout -> glibc-locale-posix (sub of glibc) -|
^                                                                                                       |
|-----------------------------------------<<<---------------<

we get:

glibc -> wolfi-baselayout -> locale-posix (sub of wolfi-baselayout)

Or just a separate package that both depend upon

wolfi-baselayout -> locale-posix
glibc -> locale-posix

I can do any of the above; point me?

@deitch
Copy link
Contributor Author

deitch commented Jun 21, 2023

I just dug into the glibc-locale-posix, and I don't see a sane way to extract it to its own package as it is built from glibc.

Which brings me back to the suggestion of @kaniini

We could remove the posix dependency from the baselayout, instead making it a runtime dep of glibc. What do you think?

Looks like the best path. I will modify this PR to reflect that, and we can comment from there.

Signed-off-by: Avi Deitcher <avi@deitcher.net>
@deitch deitch force-pushed the glibc-remove-wolfi-baselayout branch from 00bd72a to 5c347fa Compare June 21, 2023 16:46
@deitch
Copy link
Contributor Author

deitch commented Jun 21, 2023

This has been updated such that:

  • wolfi-baselayout no longer depends on glibc-locale-posix (and hence glibc, eliminating the cycle)
  • glibc runtime continues to depend on wolfi-baselayout
  • glibc runtime also depends on glibc-locale-posix

The net effects are:

  • glibc: identical
  • wolfi-baselayout: cycle is removed, but it no longer includes the locale files
  • packages that depended on wolfi-baselayout to get glibc-locale-posix (are there any? 🤷‍♂️ ) should include it explicitly, unless they also include glibc of course

Feedback from the commenters above (@kaniini @imjasonh @dlorenc)?

@kaniini
Copy link
Contributor

kaniini commented Jun 21, 2023

I think its fine to proceed with this.

@deitch
Copy link
Contributor Author

deitch commented Jun 21, 2023

Sounds good. Anything downstream we need to update?

@kaniini
Copy link
Contributor

kaniini commented Jun 21, 2023

I can't think of anything. The only staticly linked binaries we ship are Golang ones, which do not require the locale data.

@deitch
Copy link
Contributor Author

deitch commented Jun 21, 2023

Cool thanks. I'll add this one to the queue, then head back to the other issue to try and resolve the remaining cycles.

@deitch deitch added this pull request to the merge queue Jun 21, 2023
Merged via the queue into wolfi-dev:main with commit cdbb30c Jun 21, 2023
@deitch deitch deleted the glibc-remove-wolfi-baselayout branch June 21, 2023 19:49
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