feat(mk): add makefiles to allow building deps outside docker#61
feat(mk): add makefiles to allow building deps outside docker#61alexandermerritt wants to merge 5 commits into
Conversation
|
|
||
| $(XEN_SRC): | ||
| git clone -b $(XEN_BRANCH) $(XEN_URL) $@ | ||
|
|
There was a problem hiding this comment.
I'm fine doing all this in make but we should delete the equivalent steps from the Dockerfiles and just invoke these make targets directly from the respective Dockerfiles in this PR, otherwise we risk adding two parallel sets of setup/install/clone steps - and variables - that inevitably will drift, since CI only exercises the Dockerfiles as an entrypoint.
There was a problem hiding this comment.
Ditto - we should go ahead and make remove the duplicated steps from the Dockerfile and use the corresponding makefiles in the respective Dockerfiles.
Also, I think we should use the full component name in the makefile name -> Makefile.oxenstored et al. Not only is this more obvious/discoverable if you're just looking at the repo, it maintains a 1-1 mapping between e.g. Dockerfile.oxenstored and Makefile.oxenstored in the tree, which is also more obvious/discoverable.
There was a problem hiding this comment.
use the full component name in the makefile name
Agreed, will update.
| @@ -0,0 +1,122 @@ | |||
| .PHONY: all | |||
There was a problem hiding this comment.
Ditto
Makefile.qemu-xenand dedupe the Dockerfile.
| git clone -b $(XEN_BRANCH) $(XEN_URL) $@ | ||
|
|
||
| .PHONY: install-deps | ||
| install-deps: |
There was a problem hiding this comment.
install-deps as written here won't work in non-apt envs, which includes the dockerfiles themselves and any non-Debian local distro anyone tries to run this against.
So I would suggest to
- Define the packageset/install cmd in the Dockerfile (where the env is fixed), and have
maketargets assume all ambient deps are already present. - Remove this and document it (as you do below) instead of encoding it as a make target, for direct, non-Docker builds - people will have to make sure the deps are present before building locally outside of Docker.
There was a problem hiding this comment.
Makes sense, I agree. This was just a hack for myself.
|
I was planning to make edits to the Docker scripts in a separate PR, is that alright? I'll update the Makefile names and remove the install-deps target. |
Signed-off-by: Alexander Merritt <alexander@edera.dev>
alexandermerritt
force-pushed
the
alexm/dev-build
branch
from
13f54e3 to
2d9da43
Compare
Make it easier to build our software locally without added environment constraints, such as Docker and OCI registries. Included readme for documentation. Signed-off-by: Alexander Merritt <alexander@edera.dev>
alexandermerritt
force-pushed
the
alexm/dev-build
branch
from
2d9da43 to
0bb9346
Compare
Actually maybe it makes sense to do that all in this PR... I'll give it a shot. |
alexandermerritt
changed the title
alexandermerritt
changed the title
alexandermerritt
changed the title
Thanks - dealer's choice - if you do create a separate PR, please do try and crosslink it with this one. You can also have multiple PRs in flight at the same time and we can just merge them in the desired order, up to you. |
... to keep it light. Signed-off-by: Costin Lupu <costin@edera.dev>
We have a singular rolling branch for Xen in our own repo. Any new patches to it should be committed to that, rather than maintained here. Similarly for QEMU, merge the 4.21 patch into our qemu-xen repo for maintenance there. This commit must coincide with an update to the qemu-xen repo we maintain, as we are removing the patch here. Signed-off-by: Alexander Merritt <alexander@edera.dev>
Signed-off-by: Alexander Merritt <alexander@edera.dev>
3 participants
Related issue: #2235