infra: remove pkgparentdir

As the semantics of FOO_DIR_PREFIX will change in the following patch, we want to remove unnecessary usage of this variable.

Signed-off-by Thomas De Schampheleire <[email protected]>

As Arnout suggested, pkgparentdir is no longer really needed. Its usage can be replaced with pkgdir. This patch makes that change, and removes the definition of pkgparentdir.

Signed-off-by Thomas De Schampheleire <[email protected]>

---
Note: as a consequence of this change, the definition of FOO_DIR_PREFIX slightly changed. I could have renamed the variable, but FOO_DIR was already taken, and it's still more or less a prefix.

  package/pkg-autotools.mk |   6 +++---
  package/pkg-cmake.mk     |   6 +++---
  package/pkg-generic.mk   |  10 +++++-----
  package/pkg-utils.mk     |   1 -
  4 files changed, 11 insertions(+), 12 deletions(-)

package/pkg-autotools.mk +3 -3

@@ -48,7 +48,7 @@ endef
  #             for host packages
  #  argument 3 is the uppercase package name, without the HOST_ prefix
  #             for host packages
-#  argument 4 is the package directory prefix
+#  argument 4 is the package directory
  #  argument 5 is the type (target or host)
  ################################################################################
 

@@ -311,5 +311,5 @@ endef
  # autotools-package -- the target generator macro for autotools packages
  ################################################################################
 
-autotools-package = $(call inner-autotools-package,$(call pkgname),$(call UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),target)
-host-autotools-package = $(call inner-autotools-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),host)
+autotools-package = $(call inner-autotools-package,$(call pkgname),$(call UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),target)
+host-autotools-package = $(call inner-autotools-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),host)

package/pkg-cmake.mk +3 -3

@@ -31,7 +31,7 @@
  #             for host packages
  #  argument 3 is the uppercase package name, without the HOST_ prefix
  #             for host packages
-#  argument 4 is the package directory prefix
+#  argument 4 is the package directory
  #  argument 5 is the type (target or host)
  ################################################################################
 

@@ -179,8 +179,8 @@ endef
  # cmake-package -- the target generator macro for CMake packages
  ################################################################################
 
-cmake-package = $(call inner-cmake-package,$(call pkgname),$(call UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),target)
-host-cmake-package = $(call inner-cmake-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),host)
+cmake-package = $(call inner-cmake-package,$(call pkgname),$(call UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),target)
+host-cmake-package = $(call inner-cmake-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),host)
 
  ################################################################################
  # Generation of the CMake toolchain file

package/pkg-generic.mk +2 -2

@@ -89,7 +89,7 @@ endif
  # find the package directory (typically package/<pkgname>) and the
  # prefix of the patches
  $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION)
-$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME)
+$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME)
  $(BUILD_DIR)/%/.stamp_patched:
  	@$(call MESSAGE,"Patching")
  	$(foreach hook,$($(PKG)_PRE_PATCH_HOOKS),$(call $(hook))$(sep))

@@ -197,7 +197,7 @@ endif
  #             for host packages
  #  argument 3 is the uppercase package name, without the HOST_ prefix
  #             for host packages
-#  argument 4 is the package directory prefix
+#  argument 4 is the package directory
  #  argument 5 is the type (target or host)
  ################################################################################
 

package/pkg-generic.mk +2 -2

  $(2)_KCONFIG_VAR = BR2_TARGET_$(2)
  else
  $(2)_KCONFIG_VAR = BR2_PACKAGE_$(2)

@@ -565,8 +565,8 @@ endef # inner-generic-package
  ################################################################################
 
  # In the case of target packages, keep the package name "pkg"
-generic-package = $(call inner-generic-package,$(call pkgname),$(call UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),target)
+generic-package = $(call inner-generic-package,$(call pkgname),$(call UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),target)
  # In the case of host packages, turn the package name "pkg" into "host-pkg"
-host-generic-package = $(call inner-generic-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),host)
+host-generic-package = $(call inner-generic-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),host)
 
  # :mode=makefile:

package/pkg-utils.mk +0 -1

@@ -60,7 +60,6 @@ endef
  # package, for which the package directory is an empty string.
  pkgdir       = $(dir $(lastword $(MAKEFILE_LIST)))
  pkgname      = $(lastword $(subst /, ,$(call pkgdir)))
-pkgparentdir = $(patsubst %$(call pkgname)/,%,$(call pkgdir))
 
  # Define extractors for different archive suffixes
  INFLATE.bz2  = $(BZCAT)

Quoted 6 lines · Thomas De Schampheleire

As Arnout suggested, pkgparentdir is no longer really needed. Its usage can
be replaced with pkgdir. This patch makes that change, and removes the definition of pkgparentdir.

Quoted 7 lines

Signed-off-by Thomas De Schampheleire <[email protected]>

---
Note: as a consequence of this change, the definition of FOO_DIR_PREFIX slightly changed. I could have renamed the variable, but FOO_DIR was already
taken, and it's still more or less a prefix.

I think it's a better idea to choose a new name - if only to help people
who are using this variable in custom packages. Or better yet, remove it completely - it's anyway not used anymore.

Quoted 99 lines

  package/pkg-autotools.mk |   6 +++---
  package/pkg-cmake.mk     |   6 +++---
  package/pkg-generic.mk   |  10 +++++-----
  package/pkg-utils.mk     |   1 -
  4 files changed, 11 insertions(+), 12 deletions(-)

package/pkg-autotools.mk +1 -2

@@ -48,7 +48,7 @@ endef
  #             for host packages
  #  argument 3 is the uppercase package name, without the HOST_ prefix
  #             for host packages
-#  argument 4 is the package directory prefix
+#  argument 4 is the package directory
  #  argument 5 is the type (target or host)
 
 ################################################################################

@@ -311,5 +311,5 @@ endef
  # autotools-package -- the target generator macro for autotools packages
 
 ################################################################################
 
-autotools-package = $(call inner-autotools-package,$(call pkgname),$(call

UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),target)
-host-autotools-package = $(call inner-autotools-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),host)
+autotools-package = $(call inner-autotools-package,$(call pkgname),$(call UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),target)
+host-autotools-package = $(call inner-autotools-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),host)

package/pkg-cmake.mk +1 -2

@@ -31,7 +31,7 @@
  #             for host packages
  #  argument 3 is the uppercase package name, without the HOST_ prefix
  #             for host packages
-#  argument 4 is the package directory prefix
+#  argument 4 is the package directory
  #  argument 5 is the type (target or host)
 
 ################################################################################

@@ -179,8 +179,8 @@ endef
  # cmake-package -- the target generator macro for CMake packages
 
 ################################################################################
 
-cmake-package = $(call inner-cmake-package,$(call pkgname),$(call

UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),target)
-host-cmake-package = $(call inner-cmake-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgparentdir),host)
+cmake-package = $(call inner-cmake-package,$(call pkgname),$(call UPPERCASE,$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),target)
+host-cmake-package = $(call inner-cmake-package,host-$(call pkgname),$(call UPPERCASE,host-$(call pkgname)),$(call UPPERCASE,$(call pkgname)),$(call pkgdir),host)

################################################################################
# Generation of the CMake toolchain file

package/pkg-generic.mk +0 -1

@@ -89,7 +89,7 @@ endif
  # find the package directory (typically package/<pkgname>) and the
  # prefix of the patches
  $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION)
-$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS =

$($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME) +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME)
$(BUILD_DIR)/%/.stamp_patched:

        @$(call MESSAGE,"Patching")
        $(foreach hook,$($(PKG)_PRE_PATCH_HOOKS),$(call $(hook))$(sep))

package/pkg-generic.mk +1 -1

@@ -197,7 +197,7 @@ endif
  #             for host packages
  #  argument 3 is the uppercase package name, without the HOST_ prefix
  #             for host packages
-#  argument 4 is the package directory prefix
+#  argument 4 is the package directory
  #  argument 5 is the type (target or host)
 
 ################################################################################

package/pkg-generic.mk endif · line 478

Quoted 6 lines

package/pkg-generic.mk +1 -1

@@ -475,7 +475,7 @@ endif
  # kernel case, the bootloaders case, and the normal packages case.
  ifeq ($(1),linux)
  $(2)_KCONFIG_VAR = BR2_LINUX_KERNEL
-else ifeq ($(4),boot/)
+else ifneq ($(filter boot/%,$(4)),)

Since this is the only place where $(4) is used, we could use $(pkgdir)
directly and drop the 4th parameter. (I just tested and it seems to work.)

Quoted 3 lines

I think it's a better idea to choose a new name - if only to help
people who are using this variable in custom packages. Or better yet, remove it completely - it's anyway not used anymore.

The only remaining place is in the foo-patch block, to get the location of patches for that package.
That is outside the inner-generic-package, so we cannot use $(4) anymore. How do you propose to solve that?

Dear Thomas De Schampheleire,

Quoted 8 lines · Thomas De Schampheleire

Quoted 3 lines

I think it's a better idea to choose a new name - if only to help
people who are using this variable in custom packages. Or better yet, remove it completely - it's anyway not used anymore.

The only remaining place is in the foo-patch block, to get the location of patches for that package.
That is outside the inner-generic-package, so we cannot use $(4) anymore. How do you propose to solve that?

Currently:

<foo>_DIR => build directory

<foo>_DIR_PREFIX => prefix of the package directory in Buildroot
sources

<foo>_DL_DIR => location of the package download directory, for
git/cvs downloads (apparently not used consistently)

I think it would make a lot more sense to have:

<foo>_DIR => points to the package directory in Buildroot sources, e.g
package/busybox/ for Busybox. This would allow packages to easily reference their own directory, to get access to configuration files and others, instead of having to know they are located in package/busybox/. This could also be used in the patching step instead of the DIR_PREFIX thing.

<foo>_BUILDDIR => points to the package build directory in $(BUILD_DIR)

<foo>_SRCDIR => points to the package source directory (currently
identical to <foo>_BUILDDIR, would change when we introduce OOT build for packages). Actually, more than half of the patch set I have to introduce OOT is about using <foo>_SRCDIR vs <foo>_BUILDDIR in the right places.

<foo>_DL_DIR => same as before.

Hi Thomas,

Quoted 31 lines · Thomas Petazzoni

Dear Thomas De Schampheleire,

Quoted 8 lines · Thomas De Schampheleire

Quoted 3 lines

I think it's a better idea to choose a new name - if only to help
people who are using this variable in custom packages. Or better yet, remove it completely - it's anyway not used anymore.

The only remaining place is in the foo-patch block, to get the location of patches for that package.
That is outside the inner-generic-package, so we cannot use $(4) anymore. How do you propose to solve that?

Currently:

<foo>_DIR => build directory

<foo>_DIR_PREFIX => prefix of the package directory in Buildroot
sources

<foo>_DL_DIR => location of the package download directory, for
git/cvs downloads (apparently not used consistently)

I think it would make a lot more sense to have:

<foo>_DIR => points to the package directory in Buildroot sources, e.g
package/busybox/ for Busybox. This would allow packages to easily reference their own directory, to get access to configuration files and others, instead of having to know they are located in package/busybox/. This could also be used in the patching step instead of the DIR_PREFIX thing.

Suppose we change this definition, do you suggest to change all .mk files that currently reference package/foo to use FOO_DIR instead? While FOO_DIR is useful for the infrastructure, like in the patching case, I don't think it is necessary to change a file package/foo/foo.mk to remove the hardcoded package/foo inside the file. The likelihood of files being moved is very small, plus it will be very hard to enforce consistent usage of FOO_DIR in favor of package/foo in this case.

Quoted 11 lines

  <foo>_BUILDDIR => points to the package build directory in $(BUILD_DIR)

<foo>_SRCDIR => points to the package source directory (currently
identical to <foo>_BUILDDIR, would change when we introduce OOT build for packages). Actually, more than half of the patch set I have to introduce OOT is about using <foo>_SRCDIR vs <foo>_BUILDDIR in the right places.

Quoted 1 line

<foo>_DL_DIR => same as before.

In general, I think the new variable names make sense.

Best regards,
Thomas

Quoted 39 lines · Thomas De Schampheleire

Hi Thomas,

Quoted 34 lines · Thomas Petazzoni

Dear Thomas De Schampheleire,

Quoted 9 lines · Thomas De Schampheleire

Quoted 3 lines

I think it's a better idea to choose a new name - if only to help
people who are using this variable in custom packages. Or better yet, remove it completely - it's anyway not used anymore.

The only remaining place is in the foo-patch block, to get the location of patches for that package.
That is outside the inner-generic-package, so we cannot use $(4) anymore. How do you propose to solve that?

Currently:

<foo>_DIR => build directory

<foo>_DIR_PREFIX => prefix of the package directory in Buildroot
sources

<foo>_DL_DIR => location of the package download directory, for
git/cvs downloads (apparently not used consistently)

I think it would make a lot more sense to have:

<foo>_DIR => points to the package directory in Buildroot sources, e.g
package/busybox/ for Busybox. This would allow packages to easily reference their own directory, to get access to configuration files and others, instead of having to know they are located in package/busybox/. This could also be used in the patching step instead of the DIR_PREFIX thing.

As I mentioned before, I would prefer to not reuse the same name with a
different meaning, because it may confuse users who have custom modifications that rely on that variable. So instead I'd remove the <foo>_DIR completely (so that it's pretty clear that this variable is empty if anybody still uses it), and instead use e.g. <foo>_BR_DIR for this purpose.

Quoted 42 lines · Arnout Vandecappelle

Quoted 39 lines · Thomas De Schampheleire

Hi Thomas,

Quoted 34 lines · Thomas Petazzoni

Dear Thomas De Schampheleire,

Quoted 9 lines · Thomas De Schampheleire

Quoted 3 lines

I think it's a better idea to choose a new name - if only to help
people who are using this variable in custom packages. Or better yet, remove it completely - it's anyway not used anymore.

The only remaining place is in the foo-patch block, to get the location of patches for that package.
That is outside the inner-generic-package, so we cannot use $(4) anymore. How do you propose to solve that?

Currently:

<foo>_DIR => build directory

<foo>_DIR_PREFIX => prefix of the package directory in Buildroot
sources

<foo>_DL_DIR => location of the package download directory, for
git/cvs downloads (apparently not used consistently)

I think it would make a lot more sense to have:

<foo>_DIR => points to the package directory in Buildroot sources, e.g
package/busybox/ for Busybox. This would allow packages to easily reference their own directory, to get access to configuration files and others, instead of having to know they are located in package/busybox/. This could also be used in the patching step instead of the DIR_PREFIX thing.

Quoted 6 lines

As I mentioned before, I would prefer to not reuse the same name with a
different meaning, because it may confuse users who have custom modifications that rely on that variable. So instead I'd remove the <foo>_DIR completely (so that it's pretty clear that this variable is empty if anybody still uses it), and instead use e.g. <foo>_BR_DIR for this purpose.

In that case I would prefer something like <foo>_PKG_DIR, instead of BR.

Quoted 9 lines

Quoted 8 lines

Quoted 1 line

Quoted 1 line

"Arnout" == Arnout Vandecappelle <[email protected]> writes:

As I mentioned before, I would prefer to not reuse the same name with a different meaning, because it may confuse users who have custom modifications that rely on that variable. So instead I'd remove the <foo>_DIR completely (so that it's pretty clear that this variable is empty if anybody still uses it), and instead use e.g. <foo>_BR_DIR for this purpose.

Quoted 1 line

In that case I would prefer something like <foo>_PKG_DIR, instead of BR.

OK.

But for consistency with BUILDDIR and SRCDIR, I'd prefer PKGDIR.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Arnout" == Arnout Vandecappelle <[email protected]> writes:

How is that hard?

I would do what we usually do for new policy: enforce it for new
packages, but leave it alone for existing packages until someone grows tired of the inconsistency and sends a global fixup patch. That also has the advantage that we can still change our mind on the short term without needing to change things all over the place again.

That said, I think that using package/foo in the .mk file is a lot
clearer than $(FOO_XXXDIR).

Quoted 3 lines

Quoted 1 line

<foo>_DL_DIR => same as before.

In general, I think the new variable names make sense.

+1.

Quoted 14 lines

Quoted 12 lines

  <foo>_BUILDDIR => points to the package build directory in $(BUILD_DIR)

<foo>_SRCDIR => points to the package source directory (currently
identical to <foo>_BUILDDIR, would change when we introduce OOT build for packages). Actually, more than half of the patch set I have to introduce OOT is about using <foo>_SRCDIR vs <foo>_BUILDDIR in the right places.

<foo>_DL_DIR => same as before.

Quoted 1 line

In general, I think the new variable names make sense.

+1.

ThomasP, perhaps you can resubmit the patches for <foo>_SRCDIR vs
<foo>_BUILDDIR, since they can be applied independently of OOT.

Thomas,

On Tue, Nov 12, 2013 at 2:42 PM, Arnout Vandecappelle <[email protected]> wrote:
[..]

Quoted 19 lines

Quoted 11 lines

Quoted 11 lines

  <foo>_BUILDDIR => points to the package build directory in $(BUILD_DIR)

<foo>_SRCDIR => points to the package source directory (currently
identical to <foo>_BUILDDIR, would change when we introduce OOT build for packages). Actually, more than half of the patch set I have to introduce OOT is about using <foo>_SRCDIR vs <foo>_BUILDDIR in the right places.

Quoted 4 lines

Quoted 1 line

<foo>_DL_DIR => same as before.

In general, I think the new variable names make sense.

+1.

Quoted 3 lines

ThomasP, perhaps you can resubmit the patches for <foo>_SRCDIR vs
<foo>_BUILDDIR, since they can be applied independently of OOT.

Thomas, could you resubmit or point me to these patches? Is it work in progress or pretty much ready?

I'm going to take up this patch series during this release cycle...

Quoted 16 lines · Arnout Vandecappelle

Quoted 16 lines · Thomas De Schampheleire

Quoted 13 lines

Quoted 13 lines

As Arnout suggested, pkgparentdir is no longer really needed. Its usage can
be replaced with pkgdir. This patch makes that change, and removes the definition of pkgparentdir.

Signed-off-by Thomas De Schampheleire<[email protected]>

---
Note: as a consequence of this change, the definition of FOO_DIR_PREFIX slightly changed. I could have renamed the variable, but FOO_DIR was already
taken, and it's still more or less a prefix.

Quoted 3 lines

Quoted 3 lines

I think it's a better idea to choose a new name - if only to help people
who are using this variable in custom packages. Or better yet, remove it completely - it's anyway not used anymore.

The only remaining place is in the foo-patch block, to get the location of patches for that package.
That is outside the inner-generic-package, so we cannot use $(4) anymore. How do you propose to solve that?