Skip to contentSkip to conversation

package/environment-setup: replace variable KERNELDIR with KERNEL_DIR

PATCH Superseded Nayab Sayed via buildroot 2 participants 4 messages Jan 24, 2024 – Jan 31, 2024

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Nayab Sayed via buildroot

author
#0 lore raw

PATCH 1/1 package/environment-setup: replace variable KERNELDIR with KERNEL_DIR

This commit updates the host environment setup helper script to use the more conventional variable name KERNEL_DIR instead of KERNELDIR. The change is made for consistency and to align with established conventions in the project (like KERNEL_HOST, HOST_DIR etc.). No functional changes are introduced by this commit.

Signed-off-by Nayab Sayed <[email protected]>
---
 package/environment-setup/environment-setup    | 2 +-
 package/environment-setup/environment-setup.mk | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
package/environment-setup/environment-setup +1 -1
index 84a9843c24..8504a89743 100644
@@ -10,7 +10,7 @@ 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, KERNEL_DIR) are exported
 * To configure do "./configure $CONFIGURE_FLAGS" or use
   the "configure" alias
 * To build CMake-based projects, use the "cmake" alias
package/environment-setup/environment-setup.mk +1 -1
index 29ec5a9a95..cc058a5799 100644
@@ -34,7 +34,7 @@ define HOST_ENVIRONMENT_SETUP_INSTALL_CMDS
 		>> $(ENVIRONMENT_SETUP_FILE)
 
 	$(if $(BR2_LINUX_KERNEL),\
-		printf "export \"KERNELDIR=$(LINUX_BUILDDIR)\"\n" \
+		printf "export \"KERNEL_DIR=$(LINUX_BUILDDIR)\"\n" \
 			>> $(ENVIRONMENT_SETUP_FILE),)
 endef

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

Best regards,

Signature · 6 lines
Nayab Sayed <[email protected]>

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
author
#1 lore raw
Quoted 5 lines · Nayab Sayed via buildroot

This commit updates the host environment setup helper script to use the more conventional variable name KERNEL_DIR instead of KERNELDIR. The change is made for consistency and to align with established conventions in the project (like KERNEL_HOST, HOST_DIR etc.). No functional changes are introduced by this commit.

Do you have some evidence of this established convention?

We currently have three external module packages that use KERNELDIR
(falcosecurity-libs, ti-gfx and ti-sgx-km) and two that use KERNEL_DIR (kernel-module-imx-gpu-viv and gcnano-binaries).

It has been KERNELDIR in Buildroot for more than 3 years now. If we change it
now, in all likelihood we will break a number of scripts that rely on that environment variable. So I think we need quite overwhelming evidence that KERNEL_DIR is better before we introduce such breakage.

Angelo, you originally introduced KERNELDIR, anything to add to this discussion?

Regards,
Arnout

Quoted 38 lines
Signed-off-by Nayab Sayed <[email protected]>
---
  package/environment-setup/environment-setup    | 2 +-
  package/environment-setup/environment-setup.mk | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
package/environment-setup/environment-setup +1 -1
index 84a9843c24..8504a89743 100644
@@ -10,7 +10,7 @@ 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, KERNEL_DIR) are exported
  * To configure do "./configure $CONFIGURE_FLAGS" or use
    the "configure" alias
  * To build CMake-based projects, use the "cmake" alias
package/environment-setup/environment-setup.mk +1 -2
index 29ec5a9a95..cc058a5799 100644
@@ -34,7 +34,7 @@ define HOST_ENVIRONMENT_SETUP_INSTALL_CMDS
  		>> $(ENVIRONMENT_SETUP_FILE)
 
  	$(if $(BR2_LINUX_KERNEL),\
-		printf "export \"KERNELDIR=$(LINUX_BUILDDIR)\"\n" \
+		printf "export \"KERNEL_DIR=$(LINUX_BUILDDIR)\"\n" \
  			>> $(ENVIRONMENT_SETUP_FILE),)
  endef
 
 
---

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

Quoted 1 line

Best regards,

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#2 lore raw

Il mar 30 gen 2024, 21:03 Arnout Vandecappelle via buildroot < [email protected]> ha scritto:

Quoted 19 lines · Nayab Sayed via buildroot
Quoted 1 line

This commit updates the host environment setup helper script to use the

more

Quoted 1 line

conventional variable name KERNEL_DIR instead of KERNELDIR. The change

is made

Quoted 1 line

for consistency and to align with established conventions in the project

(like

Quoted 2 lines

KERNEL_HOST, HOST_DIR etc.). No functional changes are introduced by this commit.

Do you have some evidence of this established convention?

We currently have three external module packages that use KERNELDIR
(falcosecurity-libs, ti-gfx and ti-sgx-km) and two that use KERNEL_DIR (kernel-module-imx-gpu-viv and gcnano-binaries).

Quoted 9 lines

It has been KERNELDIR in Buildroot for more than 3 years now. If we
change it
now, in all likelihood we will break a number of scripts that rely on that environment variable. So I think we need quite overwhelming evidence that KERNEL_DIR is better before we introduce such breakage.

Angelo, you originally introduced KERNELDIR, anything to add to this
discussion?

I took it straight from the LDD3 kernel module makefile example.

Googling around, I cannot see a definitive answer, probably exporting both could be a nice idea.

Quoted 45 lines

Regards,
Arnout

Review

Quoted 8 lines
Signed-off-by Nayab Sayed <[email protected]>
---
  package/environment-setup/environment-setup    | 2 +-
  package/environment-setup/environment-setup.mk | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
(quoted diff) +0 -0

b/package/environment-setup/environment-setup

package/environment-setup/environment-setup cat <<'EOF' · line 13

Quoted 8 lines

index 84a9843c24..8504a89743 100644

package/environment-setup/environment-setup +0 -1
@@ -10,7 +10,7 @@ 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

package/environment-setup/environment-setup cat <<'EOF' · line 13

Quoted 1 line
package/environment-setup/environment-setup +1 -0
+* Kernel compilation variables (ARCH, CROSS_COMPILE, KERNEL_DIR) are

exported

Review

Quoted 4 lines
package/environment-setup/environment-setup +0 -0
  * To configure do "./configure $CONFIGURE_FLAGS" or use
    the "configure" alias
  * To build CMake-based projects, use the "cmake" alias
(quoted diff) +0 -0

b/package/environment-setup/environment-setup.mk

Quoted 17 lines

index 29ec5a9a95..cc058a5799 100644

package/environment-setup/environment-setup.mk +1 -2
@@ -34,7 +34,7 @@ define HOST_ENVIRONMENT_SETUP_INSTALL_CMDS
              >> $(ENVIRONMENT_SETUP_FILE)
 
      $(if $(BR2_LINUX_KERNEL),\
-             printf "export \"KERNELDIR=$(LINUX_BUILDDIR)\"\n" \
+             printf "export \"KERNEL_DIR=$(LINUX_BUILDDIR)\"\n" \
                      >> $(ENVIRONMENT_SETUP_FILE),)
  endef
 
 
---

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

Quoted 6 lines
Quoted 1 line

Best regards,

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

author
#3 lore raw
Quoted 42 lines · Angelo Compagnucci

You don't often get email from [email protected]. Learn why this is important <https://aka.ms/LearnAboutSenderIdentification>

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Il mar 30 gen 2024, 21:03 Arnout Vandecappelle via buildroot <[email protected]> ha scritto:

On 24/01/2024 06:32, Nayab Sayed via buildroot wrote:
> This commit updates the host environment setup helper script to use the more
> conventional variable name KERNEL_DIR instead of KERNELDIR.  The change is made
> for consistency and to align with established conventions in the project (like
> KERNEL_HOST, HOST_DIR etc.). No functional changes are introduced by this
> commit.

  Do you have some evidence of this established convention?

  We currently have three external module packages that use KERNELDIR (falcosecurity-libs, ti-gfx and ti-sgx-km) and two that use KERNEL_DIR
(kernel-module-imx-gpu-viv and gcnano-binaries).

  It has been KERNELDIR in Buildroot for more than 3 years now. If we change it
now, in all likelihood we will break a number of scripts that rely on that
environment variable. So I think we need quite overwhelming evidence that
KERNEL_DIR is better before we introduce such breakage.

Quoted 8 lines

  Angelo, you originally introduced KERNELDIR, anything to add to this discussion?

I took it straight from the LDD3 kernel module makefile example.

Googling around, I cannot see a definitive answer, probably exporting both could be a nice idea.

I agree keeping both would be a nice idea. I will submit patch v2 accordingly.

Quoted 58 lines

      Regards,
      Arnout

    >
    > Signed-off-by: Nayab Sayed <[email protected]>
    > ---
    >   package/environment-setup/environment-setup | 2 +-
    >   package/environment-setup/environment-setup.mk
    <http://environment-setup.mk> | 2 +-
    >   2 files changed, 2 insertions(+), 2 deletions(-)
    >
    > diff --git a/package/environment-setup/environment-setup
    b/package/environment-setup/environment-setup
    > index 84a9843c24..8504a89743 100644
    > --- a/package/environment-setup/environment-setup
    > +++ b/package/environment-setup/environment-setup
    > @@ -10,7 +10,7 @@ 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,
    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
    <http://environment-setup.mk>
    b/package/environment-setup/environment-setup.mk
    <http://environment-setup.mk>
    > index 29ec5a9a95..cc058a5799 100644
    > --- a/package/environment-setup/environment-setup.mk
    <http://environment-setup.mk>
    > +++ b/package/environment-setup/environment-setup.mk
    <http://environment-setup.mk>
    > @@ -34,7 +34,7 @@ define HOST_ENVIRONMENT_SETUP_INSTALL_CMDS
    >               >> $(ENVIRONMENT_SETUP_FILE)
    >
    >       $(if $(BR2_LINUX_KERNEL),\
    > -             printf "export \"KERNELDIR=$(LINUX_BUILDDIR)\"\n" \
    > +             printf "export \"KERNEL_DIR=$(LINUX_BUILDDIR)\"\n" \
    >                       >> $(ENVIRONMENT_SETUP_FILE),)
    >   endef
    >
    >
    > ---
    > base-commit: 04dfeff6242516a1061973fc7af2cc9c5e3dd4e2
    > change-id: 20240124-kernel_dir-41a48fd97b75
    >
    > Best regards,
    _______________________________________________
    buildroot mailing list
    [email protected]
    https://lists.buildroot.org/mailman/listinfo/buildroot
Signature · 7 lines
Thanks,
Nayab

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot