Refactor ubuntu setup #432
Conversation
cpuguy83
force-pushed
the
refactor_ubuntu_setup
branch
2 times, most recently
from
7e8bfd1
to
15a8906
Compare
frontend/deb/distro/install.go
Outdated
| apt autoclean -y | ||
|
|
||
| apt update | ||
| apt install -y $@ |
There was a problem hiding this comment.
| apt install -y $@ | |
| apt install -y "$@" |
There was a problem hiding this comment.
Quoting here would turn a list of packages into one single argument to apt which I don't think we want.
There was a problem hiding this comment.
$@ has special behavior when double-quoted, see the manual
From the manual:
That is, "$@" is equivalent to "$1" "$2" [... and so on]
cpuguy83
force-pushed
the
refactor_ubuntu_setup
branch
from
15a8906
to
6f15602
Compare
|
All updated. |
|
Needs a rebase and it's failing a test, but I'll approve once that's sorted. |
cpuguy83
force-pushed
the
refactor_ubuntu_setup
branch
from
6f15602
to
f3d5448
Compare
|
This is rebased. |
cpuguy83
force-pushed
the
refactor_ubuntu_setup
branch
from
f3d5448
to
1bd4f19
Compare
|
Rebased and comments addressed. |
Signed-off-by: Brian Goff <[email protected]>
Signed-off-by: Brian Goff <[email protected]>
This does actually end up changing the ordering, which I think is more correct. If a client has supplied a context with an image name then we should assume we need to install the distro's base build packages into it, unlike when the worker context ref is used where we expect that the provided image is the actual worker. Signed-off-by: Brian Goff <[email protected]>
This was always a bit janky. We really can't guarentee that all implementations package repos from a worker to install images into a target, and this work-around was just to make the test pass. Instead the test is now just deleted since it doesn't make sense. While it is technically possible to do this on azlinux people should be using the custom repo support add extra repos. Signed-off-by: Brian Goff <[email protected]>
This allows us to have a common implementation of a bunch of the things that underpins building a deb package. Signed-off-by: Brian Goff <[email protected]>
This moves the rest of the logic from the jammy package into the distro config. Signed-off-by: Brian Goff <[email protected]>
Do not require correct ordering of constraints options for apt/deb functions. Signed-off-by: Brian Goff <[email protected]>
cpuguy83
force-pushed
the
refactor_ubuntu_setup
branch
from
1bd4f19
to
d1fec18
Compare
|
Rebased again. |
There was a problem hiding this comment.
I think this looks good to me -- the way the functionality is encapsulated seems very clean and reusable. Personally, I am not too concerned about the parameter ordering issue. To me it makes sense that in the Install run options the constraints attached to the exec op will be applied to the child operations first, after which user specified constraints will be applied to potentially override
frontend/deb/distro/container.go
Outdated
| tmp := llb.Scratch().File(llb.Mkfile("tmp", 0o644, nil), opts...) | ||
| // Warning: HACK here | ||
| // The base ubuntu image has this `excludes` config file which prevents | ||
| // installation of a lot of thigns, including doc files. |
There was a problem hiding this comment.
| // installation of a lot of thigns, including doc files. | |
| // installation of a lot of things, including doc files. |
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| solveT(ctx, t, gwc, sr) |
There was a problem hiding this comment.
q: why is this deleted, the res.SingleRef() and err handling? Shouldn't we still do that?
There was a problem hiding this comment.
I don't think that's necessary.
If there's a problem with the build solveT will get it.
We don't need to check the actual content of the resulting image since this test is no longer making the assumption that the worker repos will be available at install time.
There was a problem hiding this comment.
This is mentioned in the commit where the change is at, btw.
There was a problem hiding this comment.
oh ok thanks, yeah, I reviewed the whole PR
Signed-off-by: Brian Goff <[email protected]>
cpuguy83
force-pushed
the
refactor_ubuntu_setup
branch
from
d1fec18
to
7a16df9
Compare
Extracts all the jammy logic into reusable components.
See individual commits for more details.