Cleanup of various scripts#148
Open
jnoordsij wants to merge 4 commits intoServerContainers:masterfrom
Open
Conversation
Contributor
Author
|
I've not been able yet to extensively test all variants, but I think they should mostly be behaving the same. But probably merging this together with the (indirect) Alpine 3.21 bump is probably preferable to ensure any breakages coincide with a new Alpine version, so people are less likely to be surprised by the impact. |
Member
|
Hi there, thanks for your work - I need to check this before I merge - to many users and it might break something. |
Contributor
Author
|
I fully understand, make sure to check it thoroughly! I think it should function equivalently and only supppress some error messages, but there's definitely chance of me having missed something. Note I've deliberately used separate commits which might help in reviewing some changes individually. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When looking into #147 and wondering whether or not the
smbd-onlytag would be suitable for our usecase, I noticed that when using this variant I was faced with various Avahi-related warnings when booting this image, making me wonder whether or not I had it properly configured. However, it seems like they are caused by a combination of:AVAHI_DISABLEnot being set in the images (this was missing fromgenerate-variants.shAVAHI_DISABLEis setI figured this was caused by some mismatching between logic in
build.shandgenerate-variants.sh. Therefore I've opted to merge the scripts, using an argument-keyword-detection system ingenerate-variants.shto distinguish "generate" and "generate+build" mode. Furthermore, I've opted to disable some additional logic in the entrypoint script by checking forAVAHI_DISABLE.Finally, I ran into some issues with
generate-variantsbecoming slower on repeated runs, due to nested tar'ing of thevariantsdirectory, which I removed by using.dockerignoreas ignore-file and adding some additional files/folders to it.For reference, the warnings can be reproduced with e.g.:
docker run --rm ghcr.io/servercontainers/samba:smbd-only-a3.20.3-s4.19.9-r0 /bin/sh -c "echo 'debug done'":After this MR the output is as follows: