Re: [Buildroot] [PATCHv4 1/1] package/heimdal: upgrade package and add target support

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

Hello Guillaume,

On Mon, 28 Apr 2025 11:40:55 -0400
Guillaume Chaye <guillaume.chaye@zeetim.com> wrote:

> This patch does several things, so I will try to explain clearly what has been modified.
> 
> First, it bumps Heimdal to the latest commit (16/04/2025).
> The current version was more than 3 years old and had known CVEs (see CVE-2023-36328).
> This upgrade allows the removal of the two patches currently applied:
> - The commit 1b57b62, which fixes the build with autoconf 2.72, is already included upstream.
> - We no longer need to use the JSON-PP module, as it is now optional (commit 13d3bcf).
> 
> Heimdal was previously a host-only package. This patch adds target support.
> This requires cross-compiling the project, which was not possible without additional patches.
> After some research, I found this pull request (https://github.com/heimdal/heimdal/pull/1174)
> created by a well-known Heimdal contributor (Nico Williams).
> This pull request contains 29 commits, which is quite large, so I have tried to keep things simple
> by selecting only the minimal set of changes required for cross-compilation.
> 
> After extensive testing, I have successfully built the target version of Heimdal by applying 3 commits
> picked from the pull request:
> - The first one adds the AX_PROG_CC_FOR_BUILD macro to the project.
>   This macro, from the GNU project (https://www.gnu.org/software/autoconf-archive/ax_prog_cc_for_build.html),
>   allows detecting a C compiler that produces native executables.
> - The second commit integrates the AX_PROG_CC_FOR_BUILD macro into Heimdal's autoconf scripts,
>   enabling proper compiler selection during the build.
> - Finally, a patch was needed to fix header errors when cross-compiling libroken.
> 
> Additionally, I have added two patches I wrote and submitted upstream:
> - The first removes a hardcoded path from a script.
> - The second fixes an error generated by the 'ar' utility during the build process.
> 
> Finally, since the latest version of Heimdal no longer depends on e2fsprogs,
> the host-e2fsprogs dependency has been removed.
> 
> Signed-off-by: Guillaume Chaye <guillaume.chaye@zeetim.com>

Thanks for this new iteration. There is still too much in there, mixing
the bump and the addition of the target package.

Since the version bump made sense on its own, I extracted just this
aspect from your patch, and merged it:

  https://gitlab.com/buildroot.org/buildroot/-/commit/573ecbd44cd16b2ca4ce71b40459bcecf8c8e98c

For the rest, could you please:

- Provide a new patch with a commit message that's wrapped at 80
  columns and doesn't use first person sentence like "I have..."

- Justify why heimdal for the target is useful, i.e what will use it.

- Double check that the AX_PROG_CC_FOR_BUILD function in
  autoconf-archive really doesn't work? This would allow to avoid
  having another copy of it.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot