[v2] package/environment-setup: add variable KERNEL_DIR alongside KERNELDIR

Update host environment setup helper script to incorporate the variable
KENREL_DIR alongside the existing KERNELDIR. This adjustment ensures
consistency, as some projects utilize KERNELDIR while others use KERNEL_DIR.

Signed-off-by: Nayab Sayed <nayabbasha.sayed@microchip.com>
---
Changes in v2:
- Keep both KERNEL_DIR and KERNELDIR.
- Update commit message
- Link to v1: https://lore.kernel.org/r/20240124-kernel_dir-v1-1-4125408cf65d@microchip.com
---
 package/environment-setup/environment-setup    | 3 ++-
 package/environment-setup/environment-setup.mk | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)


---
base-commit: 04dfeff6242516a1061973fc7af2cc9c5e3dd4e2
change-id: 20240124-kernel_dir-41a48fd97b75

Best regards,

On 31/01/2024 19:18, Nayab Sayed via buildroot wrote:
> Update host environment setup helper script to incorporate the variable
> KENREL_DIR alongside the existing KERNELDIR. This adjustment ensures
> consistency, as some projects utilize KERNELDIR while others use KERNEL_DIR.
>
> Signed-off-by: Nayab Sayed<nayabbasha.sayed@microchip.com>
> ---
> Changes in v2:
> - Keep both KERNEL_DIR and KERNELDIR.
> - Update commit message
> - Link to v1:https://lore.kernel.org/r/20240124-kernel_dir-v1-1-4125408cf65d@microchip.com
> ---
>   package/environment-setup/environment-setup    | 3 ++-
>   package/environment-setup/environment-setup.mk | 2 ++
>   2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/package/environment-setup/environment-setup b/package/environment-setup/environment-setup
> index 84a9843c24..0bae813e1a 100644
> --- a/package/environment-setup/environment-setup
> +++ b/package/environment-setup/environment-setup
> @@ -10,7 +10,8 @@ cat <<'EOF'
>   Some tips:
>   * PATH now contains the SDK utilities
>   * Standard autotools variables (CC, LD, CFLAGS) are exported
> -* Kernel compilation variables (ARCH, CROSS_COMPILE, KERNELDIR) are exported
> +* Kernel compilation variables (ARCH, CROSS_COMPILE, KERNELDIR or KERNEL_DIR)
> +  are exported
>   * To configure do "./configure $CONFIGURE_FLAGS" or use
>     the "configure" alias
>   * To build CMake-based projects, use the "cmake" alias
> diff --git a/package/environment-setup/environment-setup.mk b/package/environment-setup/environment-setup.mk
> index 29ec5a9a95..4a71f18f3a 100644
> --- a/package/environment-setup/environment-setup.mk
> +++ b/package/environment-setup/environment-setup.mk
> @@ -35,6 +35,8 @@ define HOST_ENVIRONMENT_SETUP_INSTALL_CMDS
>   
>   	$(if $(BR2_LINUX_KERNEL),\
>   		printf "export \"KERNELDIR=$(LINUX_BUILDDIR)\"\n" \
> +			>> $(ENVIRONMENT_SETUP_FILE)
> +		printf "export \"KERNEL_DIR=$(LINUX_BUILDDIR)\"\n" \
>   			>> $(ENVIRONMENT_SETUP_FILE),)
>   endef
>   
>
> ---
> base-commit: 04dfeff6242516a1061973fc7af2cc9c5e3dd4e2
> change-id: 20240124-kernel_dir-41a48fd97b75
>
> Best regards,

Looks good to me.


Reviewed-by: Antoine Coutant <antoine.coutant@smile.fr> 
<mailto:antoine.coutant@smile.fr>


Regards,

Antoine Coutant.

Hello Nayab,

On Wed, 31 Jan 2024 23:48:28 +0530
Nayab Sayed via buildroot <buildroot@buildroot.org> wrote:

> Update host environment setup helper script to incorporate the variable
> KENREL_DIR alongside the existing KERNELDIR. This adjustment ensures
> consistency, as some projects utilize KERNELDIR while others use KERNEL_DIR.
> 
> Signed-off-by: Nayab Sayed <nayabbasha.sayed@microchip.com>

Thanks for this new iteration, but on my side, I'm tempted to reject
this patch, and actually to remove the support for KERNELDIR as well.

Indeed, even Angelo, who added the environment-setup script was not
able to provide a solid reason for exporting KERNELDIR in the first
place. I don't think there is any standard, or somewhat standard
variable to point to the Linux kernel source directory.

I'm fine with environment-setup exposing CC, LD, CFLAGS, LDFLAGS, or
even CROSS_COMPILE as these are reasonably standardized by various
build systems. But KERNELDIR or KERNEL_DIR are not, and the simple fact
that both would be needed/useful is a clear hint of that. The whole
thing I didn't like in the first place with environment-setup is the
fact that it exports a mixed bag of somewhat random environment
variables, and exposing both KERNELDIR and KERNEL_DIR clearly goes
further into this direction in my opinion.

Best regards,

Thomas

On 12/02/2024 22:59, Thomas Petazzoni wrote:
> Hello Nayab,
> 
> On Wed, 31 Jan 2024 23:48:28 +0530
> Nayab Sayed via buildroot <buildroot@buildroot.org> wrote:
> 
>> Update host environment setup helper script to incorporate the variable
>> KENREL_DIR alongside the existing KERNELDIR. This adjustment ensures
>> consistency, as some projects utilize KERNELDIR while others use KERNEL_DIR.
>>
>> Signed-off-by: Nayab Sayed <nayabbasha.sayed@microchip.com>
> 
> Thanks for this new iteration, but on my side, I'm tempted to reject
> this patch, and actually to remove the support for KERNELDIR as well.
> 
> Indeed, even Angelo, who added the environment-setup script was not
> able to provide a solid reason for exporting KERNELDIR in the first
> place. I don't think there is any standard, or somewhat standard
> variable to point to the Linux kernel source directory.
> 
> I'm fine with environment-setup exposing CC, LD, CFLAGS, LDFLAGS, or
> even CROSS_COMPILE as these are reasonably standardized by various
> build systems. But KERNELDIR or KERNEL_DIR are not, and the simple fact
> that both would be needed/useful is a clear hint of that. The whole
> thing I didn't like in the first place with environment-setup is the
> fact that it exports a mixed bag of somewhat random environment
> variables, and exposing both KERNELDIR and KERNEL_DIR clearly goes
> further into this direction in my opinion.

  Well, without an exported KERNELDIR, it will be very hard to build kernel 
modules outside of buildroot (and the whole point of this environment-setup 
thing is to make it easier to build stuff outside of buildroot). Indeed, 
output/build/linux-* will match stuff like linux-pam as well.

  That said, I agree that exporting both KERNELDIR and KERNEL_DIR sounds a bit 
over the top.

  Regards,
  Arnout