-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add x86-64_full capability #60
Conversation
…of dnsmasq Signed-off-by: DL6ER <[email protected]>
The action on FTL's branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we move all the extra stuff that is needed only for the all-in
below the FROM builder AS all-in-compilation
line?
So it won't be included in the pushed "normal" build image?
__
I would also add a new "tester-all-in" step to be able to push an "all-in" image without the test remains.
Creating two containers (one large one, another one even larger) doesn't seem to make too much sense when we can just live with one (the even larger). It does not only save (total) space but also eases maintenance as we'd otherwise have two mostly identical Dockerfile where we need to update/change things. The separation of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also test the "all-in" during image creation and not only during the builds in the FTL repo. Currently, our ftl-build.yml
only build two targtes, builder
and tester
.
You can see it in the latest run, the "all-in" is not tested: https://github.com/pi-hole/docker-base-images/actions/runs/3443550395/jobs/5745202463
You would need to add a section like
-
name: Build all-in
uses: docker/build-push-action@v3
with:
context: ftl-build/x86_64_full/.
push: false
target: all-in-compilation
tags: ${{ steps.meta.outputs.tags }}
labels: ${{ steps.meta.outputs.labels }}
Co-authored-by: yubiuser <[email protected]> Signed-off-by: DL6ER <[email protected]>
9d60cd0
to
5a1de90
Compare
Signed-off-by: DL6ER <[email protected]>
5a1de90
to
8e81853
Compare
Signed-off-by: yubiuser <[email protected]>
Thank you for your contribution to the Pi-hole Community!
Please read the comments below to help us consider your Pull Request.
We are all volunteers and completing the process outlined will help us review your commits quicker.
Please make sure you
What does this PR aim to accomplish?:
Allow building a (nearly) all-options build of
dnsmasq
How does this PR accomplish the above?:
Add remaining dependencies. We skip
i18n
andubus
as the former strongly depends on the user's system and makes nearly no difference code-wise (one macro is re-defined).ubus
is only natively available on OpenWRT systems and not easily installable on any other distributions.The
x86_64
container needs more time for building now as it has an additional stage to build such a fulldnsmasq
build in addition to the regular build. Both binaries are tested.Link documentation PRs if any are needed to support this PR:
None
By submitting this pull request, I confirm the following:
git rebase
)