Skip to content
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

Update ftl-build container to Alpine v3.21 #100

Merged
merged 4 commits into from
Dec 9, 2024
Merged

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Dec 9, 2024

What does this implement/fix?

Update ftl-build container to Alpine v3.21

HIGHLIGHTS:

  • Linux kernel 6.12
  • GCC 14
  • LLVM (clang) 19

As always, the major upgrade to GCC 14.2.0 is the most important reason for us to upgrade our build environment.

As it turned out that quite a few changes were necessary to support Alpine 3.12, I decided it's better to pin the container to a certain version instead of the more generic latest. It can always be updated and we can use it to tag a new container when new Alpine versions are released. The PR with the necessary changes for FTL will be opened once this PR is merged and new containers are tagged to update the used build container at the same time in the FTL PR.


Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repositories developmental branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

DL6ER added 3 commits December 9, 2024 09:52
… include the standard headers as well as unistd.h for the write function

Signed-off-by: DL6ER <[email protected]>
…s will fail with

tput: No value for $TERM and no -T specified

Signed-off-by: DL6ER <[email protected]>
@DL6ER DL6ER added the ftl-build label Dec 9, 2024
@DL6ER DL6ER requested a review from a team December 9, 2024 10:42
RUN curl -sSL https://ftl.pi-hole.net/libraries/termcap-${termcapversion}.tar.gz | tar -xz \
&& cd termcap-${termcapversion} \
&& ./configure --enable-static --disable-shared --disable-doc --without-examples \
&& sed -i '1i #define STDC_HEADERS 1\n#include <unistd.h>' termcap.c tparam.c \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patching is necessary due to a breaking change in GCC 14:

Implicit function declarations (-Werror=implicit-function-declaration)
It is no longer possible to call a function that has not been declared. In general, the solution is to include a header file with an appropriate function prototype. Note that GCC will perform further type checks based on the function prototype, which can reveal further type errors that require additional changes.

For well-known functions declared in standard headers, GCC provides fix-it hints with the appropriate #include directives:

error: implicit declaration of function ‘strlen’ [-Wimplicit-function-declaration]
   5 |   return strlen (s);
     |          ^~~~~~
note: include ‘<string.h>’ or provide a declaration of ‘strlen’
 +++ |+#include <string.h>
   1 |

On GNU systems, headers described in standards (such as the C standard, or POSIX) may require the definition of certain macros at the start of the compilation before all required function declarations are made available. See Feature Test Macros in the GNU C Library manual for details.

@yubiuser
Copy link
Member

yubiuser commented Dec 9, 2024

Maybe we should also port the monkey patch from
https://github.com/pi-hole/FTL/pull/2044/commits
for nicer BATS output

@DL6ER
Copy link
Member Author

DL6ER commented Dec 9, 2024

@yubiuser As TERM is now set by us in the tester, we get this:
image

Anything needed in addition? I don't think it needs to be "too pretty" as this isn't something we're looking often at.

@DL6ER DL6ER mentioned this pull request Dec 9, 2024
5 tasks
@yubiuser
Copy link
Member

yubiuser commented Dec 9, 2024

@@ -1,4 +1,4 @@
ARG CONTAINER="alpine:latest"
ARG CONTAINER="alpine:3.21"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@DL6ER DL6ER merged commit 4ccbd54 into master Dec 9, 2024
14 checks passed
@DL6ER DL6ER deleted the ftl-build/alpine-v3.21 branch December 10, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants