[ptxdist] [PATCH v2 0/4] ptxd_make_world_inject: Spring cleanup and optional dest dir

[ptxdist] [PATCH v2 0/4] ptxd_make_world_inject: Spring cleanup and optional dest dir

[Alexander Dahl]

Hello,

some spring cleaning and a new feature for the firmware inject
mechanism.

When trying to re-build U-Boot as an oot build for the Karo QSBASE93
Evalkit (for QS93 Solder-Down System-on-Module) it became apparent
U-Boot wants some of those binary blobs in build folder instead of
source folder.  Supporting this usecase is nice to have, because you
won't clutter your source tree, especially when building from external
tree after something like `ptxdist local-src u-boot ~/src/u-boot` …

Maybe we should add some documentation for this inject mechanism?

Follow-up patches for u-boot package in preparation already, but let me
know what you think of this first.

Greets
Alex

v2:
- rework patch 4 introducing the new inject oot mechanism
  (using the new <PKG>_INJECT_OOT variable now instead of a needlessly
  flexible approach in v1)

v1: https://lore.ptxdist.org/ptxdist/20240424143109.277373-1-ada@thorsis.com/T/#t
- initial patch stack revision (implicit)

Alexander Dahl (4):
  ptxd_make_world_inject: Remove useless test
  ptxd_make_world_inject: Use <PKG>_DIR directly
  ptxd_make_world_inject: Escape inject path and files
  ptxd_make_world_inject: Introduce new variable <PKG>_INJECT_OOT

 rules/post/ptxd_make_world_inject.make |  6 +++---
 scripts/lib/ptxd_make_world_inject.sh  | 26 ++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 7 deletions(-)


base-commit: 5a720c419cd1fabde04e8de8d1d1367c76a2666b
-- 
2.39.2

[ptxdist] [PATCH v2 1/4] ptxd_make_world_inject: Remove useless test

[Alexander Dahl]

One line above $target is set to "${var}/$(subshell)" and even if $var
_and_ the result of the subshell call is empty, there's still the slash.
Thus target is never empty.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - no change
    
    v1:
    - implicit

 scripts/lib/ptxd_make_world_inject.sh | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/scripts/lib/ptxd_make_world_inject.sh b/scripts/lib/ptxd_make_world_inject.sh
index fe4eb8363..5c2d0dc5f 100644
--- a/scripts/lib/ptxd_make_world_inject.sh
+++ b/scripts/lib/ptxd_make_world_inject.sh
@@ -11,9 +11,6 @@ ptxd_make_inject() {
 
     source="$(echo ${inject_file} | cut -d ":" -f 1)"
     target="${pkg_source}/$(echo ${inject_file} | cut -d ":" -f 2)"
-    if [ -z "${target}" ]; then
-	target="${source}"
-    fi
 
     if [[ "${source}" =~ ^/.* ]]; then
 	ptxd_bailout "'${source}' must not be an absolute path!" \
-- 
2.39.2

Re: [ptxdist] [APPLIED] ptxd_make_world_inject: Remove useless test

[Michael Olbrich]

Thanks, applied as 33fa14b8b788e2d57d015c91c4c758708824a05d.

Michael

[sent from post-receive hook]

On Sun, 12 May 2024 15:16:41 +0200, Alexander Dahl <ada@thorsis.com> wrote:
> One line above $target is set to "${var}/$(subshell)" and even if $var
> _and_ the result of the subshell call is empty, there's still the slash.
> Thus target is never empty.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> Acked-by: Michael Riesch <michael.riesch@wolfvision.net>
> Message-Id: <20240506105051.323546-2-ada@thorsis.com>
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> 
> diff --git a/scripts/lib/ptxd_make_world_inject.sh b/scripts/lib/ptxd_make_world_inject.sh
> index fe4eb8363619..5c2d0dc5fb3e 100644
> --- a/scripts/lib/ptxd_make_world_inject.sh
> +++ b/scripts/lib/ptxd_make_world_inject.sh
> @@ -11,9 +11,6 @@ ptxd_make_inject() {
>  
>      source="$(echo ${inject_file} | cut -d ":" -f 1)"
>      target="${pkg_source}/$(echo ${inject_file} | cut -d ":" -f 2)"
> -    if [ -z "${target}" ]; then
> -	target="${source}"
> -    fi
>  
>      if [[ "${source}" =~ ^/.* ]]; then
>  	ptxd_bailout "'${source}' must not be an absolute path!" \

[ptxdist] [PATCH v2 2/4] ptxd_make_world_inject: Use <PKG>_DIR directly

[Alexander Dahl]

pkg_source was defined as "$($(1)_DIR)" which is the same as pkg_dir in
ptxd_make_world_common.  We can use pkg_dir directly.  Add a safe-guard
to bail out early if that var is empty.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - no change
    
    v1:
    - implicit

 rules/post/ptxd_make_world_inject.make | 3 +--
 scripts/lib/ptxd_make_world_inject.sh  | 6 +++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/rules/post/ptxd_make_world_inject.make b/rules/post/ptxd_make_world_inject.make
index b7d28e92f..3506ee114 100644
--- a/rules/post/ptxd_make_world_inject.make
+++ b/rules/post/ptxd_make_world_inject.make
@@ -9,8 +9,7 @@
 world/inject/env = \
 	$(call world/env, $(1)) \
 	pkg_inject_path="$($(1)_INJECT_PATH)" \
-	pkg_inject_files="$($(1)_INJECT_FILES)" \
-	pkg_source="$($(1)_DIR)"
+	pkg_inject_files="$($(1)_INJECT_FILES)"
 
 world/inject = \
 	$(call world/inject/env,$(strip $(1))) \
diff --git a/scripts/lib/ptxd_make_world_inject.sh b/scripts/lib/ptxd_make_world_inject.sh
index 5c2d0dc5f..b74e464c6 100644
--- a/scripts/lib/ptxd_make_world_inject.sh
+++ b/scripts/lib/ptxd_make_world_inject.sh
@@ -10,7 +10,7 @@ ptxd_make_inject() {
     local source target
 
     source="$(echo ${inject_file} | cut -d ":" -f 1)"
-    target="${pkg_source}/$(echo ${inject_file} | cut -d ":" -f 2)"
+    target="${pkg_dir}/$(echo ${inject_file} | cut -d ":" -f 2)"
 
     if [[ "${source}" =~ ^/.* ]]; then
 	ptxd_bailout "'${source}' must not be an absolute path!" \
@@ -32,6 +32,10 @@ export -f ptxd_make_inject
 ptxd_make_world_inject() {
     ptxd_make_world_init || return
 
+    if [ -z "${pkg_dir}" ]; then
+	ptxd_bailout "<PKG>_DIR empty, no destination to inject to."
+    fi
+
     for inject_file in ${pkg_inject_files}; do
 	ptxd_make_inject || return
     done
-- 
2.39.2

Re: [ptxdist] [APPLIED] ptxd_make_world_inject: Use <PKG>_DIR directly

[Michael Olbrich]

Thanks, applied as 0eea7080662b36cd39f03a3b0785d0e4a1c346bc.

Michael

[sent from post-receive hook]

On Sun, 12 May 2024 15:16:41 +0200, Alexander Dahl <ada@thorsis.com> wrote:
> pkg_source was defined as "$($(1)_DIR)" which is the same as pkg_dir in
> ptxd_make_world_common.  We can use pkg_dir directly.  Add a safe-guard
> to bail out early if that var is empty.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> Acked-by: Michael Riesch <michael.riesch@wolfvision.net>
> Message-Id: <20240506105051.323546-3-ada@thorsis.com>
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> 
> diff --git a/rules/post/ptxd_make_world_inject.make b/rules/post/ptxd_make_world_inject.make
> index b7d28e92f25b..3506ee1140e1 100644
> --- a/rules/post/ptxd_make_world_inject.make
> +++ b/rules/post/ptxd_make_world_inject.make
> @@ -9,8 +9,7 @@
>  world/inject/env = \
>  	$(call world/env, $(1)) \
>  	pkg_inject_path="$($(1)_INJECT_PATH)" \
> -	pkg_inject_files="$($(1)_INJECT_FILES)" \
> -	pkg_source="$($(1)_DIR)"
> +	pkg_inject_files="$($(1)_INJECT_FILES)"
>  
>  world/inject = \
>  	$(call world/inject/env,$(strip $(1))) \
> diff --git a/scripts/lib/ptxd_make_world_inject.sh b/scripts/lib/ptxd_make_world_inject.sh
> index 5c2d0dc5fb3e..b74e464c6895 100644
> --- a/scripts/lib/ptxd_make_world_inject.sh
> +++ b/scripts/lib/ptxd_make_world_inject.sh
> @@ -10,7 +10,7 @@ ptxd_make_inject() {
>      local source target
>  
>      source="$(echo ${inject_file} | cut -d ":" -f 1)"
> -    target="${pkg_source}/$(echo ${inject_file} | cut -d ":" -f 2)"
> +    target="${pkg_dir}/$(echo ${inject_file} | cut -d ":" -f 2)"
>  
>      if [[ "${source}" =~ ^/.* ]]; then
>  	ptxd_bailout "'${source}' must not be an absolute path!" \
> @@ -32,6 +32,10 @@ export -f ptxd_make_inject
>  ptxd_make_world_inject() {
>      ptxd_make_world_init || return
>  
> +    if [ -z "${pkg_dir}" ]; then
> +	ptxd_bailout "<PKG>_DIR empty, no destination to inject to."
> +    fi
> +
>      for inject_file in ${pkg_inject_files}; do
>  	ptxd_make_inject || return
>      done

[ptxdist] [PATCH v2 3/4] ptxd_make_world_inject: Escape inject path and files

[Alexander Dahl]

Same as in ptxd_make_world_common for all the other variables set in
make and used in shell.

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - no change
    
    v1:
    - implicit

 rules/post/ptxd_make_world_inject.make | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rules/post/ptxd_make_world_inject.make b/rules/post/ptxd_make_world_inject.make
index 3506ee114..eabcdd052 100644
--- a/rules/post/ptxd_make_world_inject.make
+++ b/rules/post/ptxd_make_world_inject.make
@@ -8,8 +8,8 @@
 
 world/inject/env = \
 	$(call world/env, $(1)) \
-	pkg_inject_path="$($(1)_INJECT_PATH)" \
-	pkg_inject_files="$($(1)_INJECT_FILES)"
+	pkg_inject_path="$(call ptx/escape,$($(1)_INJECT_PATH))" \
+	pkg_inject_files="$(call ptx/escape,$($(1)_INJECT_FILES))"
 
 world/inject = \
 	$(call world/inject/env,$(strip $(1))) \
-- 
2.39.2

Re: [ptxdist] [APPLIED] ptxd_make_world_inject: Escape inject path and files

[Michael Olbrich]

Thanks, applied as 23c5f686998c50412b5f5fd99d7dc9c345fe1af8.

Michael

[sent from post-receive hook]

On Sun, 12 May 2024 15:16:42 +0200, Alexander Dahl <ada@thorsis.com> wrote:
> Same as in ptxd_make_world_common for all the other variables set in
> make and used in shell.
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> Acked-by: Michael Riesch <michael.riesch@wolfvision.net>
> Message-Id: <20240506105051.323546-4-ada@thorsis.com>
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> 
> diff --git a/rules/post/ptxd_make_world_inject.make b/rules/post/ptxd_make_world_inject.make
> index 3506ee1140e1..eabcdd0529b8 100644
> --- a/rules/post/ptxd_make_world_inject.make
> +++ b/rules/post/ptxd_make_world_inject.make
> @@ -8,8 +8,8 @@
>  
>  world/inject/env = \
>  	$(call world/env, $(1)) \
> -	pkg_inject_path="$($(1)_INJECT_PATH)" \
> -	pkg_inject_files="$($(1)_INJECT_FILES)"
> +	pkg_inject_path="$(call ptx/escape,$($(1)_INJECT_PATH))" \
> +	pkg_inject_files="$(call ptx/escape,$($(1)_INJECT_FILES))"
>  
>  world/inject = \
>  	$(call world/inject/env,$(strip $(1))) \

[ptxdist] [PATCH v2 4/4] ptxd_make_world_inject: Introduce new variable <PKG>_INJECT_OOT

[Alexander Dahl]

Setting the new variable to 'YES' allows to inject into <PKG>_BUILD_DIR
instead of <PKG>_DIR as before.

When building out-of-tree some bootloaders like U-Boot expect injected
files in the build dir, not in the source dir.  Backwards compatibility
is ensured, the source dir is still the default, the new inject dest is
optional and in case set it overwrites the default.

For using this in u-boot package, on top of the things already done to
use the old inject mechanism in general, add this line to
'rules/u-boot.make' for example:

    U_BOOT_INJECT_OOT     := YES

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---

Notes:
    v2:
    - dropped the previous approach with the more flexible <PKG>_INJECT_DEST
    - reworked the whole logic to use new <PKG>_INJECT_OOT variable
    
    v1:
    - implicit

 rules/post/ptxd_make_world_inject.make |  3 ++-
 scripts/lib/ptxd_make_world_inject.sh  | 23 ++++++++++++++++++++---
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/rules/post/ptxd_make_world_inject.make b/rules/post/ptxd_make_world_inject.make
index eabcdd052..1e3979a2f 100644
--- a/rules/post/ptxd_make_world_inject.make
+++ b/rules/post/ptxd_make_world_inject.make
@@ -9,7 +9,8 @@
 world/inject/env = \
 	$(call world/env, $(1)) \
 	pkg_inject_path="$(call ptx/escape,$($(1)_INJECT_PATH))" \
-	pkg_inject_files="$(call ptx/escape,$($(1)_INJECT_FILES))"
+	pkg_inject_files="$(call ptx/escape,$($(1)_INJECT_FILES))" \
+	pkg_inject_oot="$(call ptx/escape,$($(1)_INJECT_OOT))"
 
 world/inject = \
 	$(call world/inject/env,$(strip $(1))) \
diff --git a/scripts/lib/ptxd_make_world_inject.sh b/scripts/lib/ptxd_make_world_inject.sh
index b74e464c6..e8e94fbe6 100644
--- a/scripts/lib/ptxd_make_world_inject.sh
+++ b/scripts/lib/ptxd_make_world_inject.sh
@@ -10,7 +10,7 @@ ptxd_make_inject() {
     local source target
 
     source="$(echo ${inject_file} | cut -d ":" -f 1)"
-    target="${pkg_dir}/$(echo ${inject_file} | cut -d ":" -f 2)"
+    target="${inject_dest}/$(echo ${inject_file} | cut -d ":" -f 2)"
 
     if [[ "${source}" =~ ^/.* ]]; then
 	ptxd_bailout "'${source}' must not be an absolute path!" \
@@ -32,8 +32,25 @@ export -f ptxd_make_inject
 ptxd_make_world_inject() {
     ptxd_make_world_init || return
 
-    if [ -z "${pkg_dir}" ]; then
-	ptxd_bailout "<PKG>_DIR empty, no destination to inject to."
+    if [ -z "${pkg_inject_oot}" ]; then
+	pkg_inject_oot=NO
+    fi
+
+    case "${pkg_inject_oot}" in
+	"YES") inject_dest="${pkg_build_dir}" ;;
+	"NO")  inject_dest="${pkg_dir}" ;;
+	*)     ptxd_bailout "<PKG>_INJECT_OOT: please set to YES or NO" ;;
+    esac
+
+    if [ "${pkg_build_oot:-NO}" = 'NO' ] && [ "${pkg_inject_oot}" != 'NO' ]; then
+	ptxd_warning "<PKG>_BUILD_OOT and <PKG>_INJECT_OOT contradict each other." \
+	    "Using $(ptxd_print_path ${inject_dest}) as inject destination anyways."
+    fi
+
+    if [ ! -d "${inject_dest}" ]; then
+	ptxd_bailout "<PKG> inject destination dir missing." \
+	    "Correct placement of world/inject depends on <PKG>_BUILD_OOT and <PKG>_INJECT_OOT." \
+	    "Check order of calls in prepare stage!"
     fi
 
     for inject_file in ${pkg_inject_files}; do
-- 
2.39.2

Re: [ptxdist] [APPLIED] ptxd_make_world_inject: Introduce new variable <PKG>_INJECT_OOT

[Michael Olbrich]

Thanks, applied as d4caaae5e1e35470f24b8b3da79aa77b57c4deb6.

Michael

[sent from post-receive hook]

On Sun, 12 May 2024 15:16:43 +0200, Alexander Dahl <ada@thorsis.com> wrote:
> Setting the new variable to 'YES' allows to inject into <PKG>_BUILD_DIR
> instead of <PKG>_DIR as before.
> 
> When building out-of-tree some bootloaders like U-Boot expect injected
> files in the build dir, not in the source dir.  Backwards compatibility
> is ensured, the source dir is still the default, the new inject dest is
> optional and in case set it overwrites the default.
> 
> For using this in u-boot package, on top of the things already done to
> use the old inject mechanism in general, add this line to
> 'rules/u-boot.make' for example:
> 
>     U_BOOT_INJECT_OOT     := YES
> 
> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> Acked-by: Michael Riesch <michael.riesch@wolfvision.net>
> Message-Id: <20240506105051.323546-5-ada@thorsis.com>
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> 
> diff --git a/rules/post/ptxd_make_world_inject.make b/rules/post/ptxd_make_world_inject.make
> index eabcdd0529b8..1e3979a2f007 100644
> --- a/rules/post/ptxd_make_world_inject.make
> +++ b/rules/post/ptxd_make_world_inject.make
> @@ -9,7 +9,8 @@
>  world/inject/env = \
>  	$(call world/env, $(1)) \
>  	pkg_inject_path="$(call ptx/escape,$($(1)_INJECT_PATH))" \
> -	pkg_inject_files="$(call ptx/escape,$($(1)_INJECT_FILES))"
> +	pkg_inject_files="$(call ptx/escape,$($(1)_INJECT_FILES))" \
> +	pkg_inject_oot="$(call ptx/escape,$($(1)_INJECT_OOT))"
>  
>  world/inject = \
>  	$(call world/inject/env,$(strip $(1))) \
> diff --git a/scripts/lib/ptxd_make_world_inject.sh b/scripts/lib/ptxd_make_world_inject.sh
> index b74e464c6895..e8e94fbe6397 100644
> --- a/scripts/lib/ptxd_make_world_inject.sh
> +++ b/scripts/lib/ptxd_make_world_inject.sh
> @@ -10,7 +10,7 @@ ptxd_make_inject() {
>      local source target
>  
>      source="$(echo ${inject_file} | cut -d ":" -f 1)"
> -    target="${pkg_dir}/$(echo ${inject_file} | cut -d ":" -f 2)"
> +    target="${inject_dest}/$(echo ${inject_file} | cut -d ":" -f 2)"
>  
>      if [[ "${source}" =~ ^/.* ]]; then
>  	ptxd_bailout "'${source}' must not be an absolute path!" \
> @@ -32,8 +32,25 @@ export -f ptxd_make_inject
>  ptxd_make_world_inject() {
>      ptxd_make_world_init || return
>  
> -    if [ -z "${pkg_dir}" ]; then
> -	ptxd_bailout "<PKG>_DIR empty, no destination to inject to."
> +    if [ -z "${pkg_inject_oot}" ]; then
> +	pkg_inject_oot=NO
> +    fi
> +
> +    case "${pkg_inject_oot}" in
> +	"YES") inject_dest="${pkg_build_dir}" ;;
> +	"NO")  inject_dest="${pkg_dir}" ;;
> +	*)     ptxd_bailout "<PKG>_INJECT_OOT: please set to YES or NO" ;;
> +    esac
> +
> +    if [ "${pkg_build_oot:-NO}" = 'NO' ] && [ "${pkg_inject_oot}" != 'NO' ]; then
> +	ptxd_warning "<PKG>_BUILD_OOT and <PKG>_INJECT_OOT contradict each other." \
> +	    "Using $(ptxd_print_path ${inject_dest}) as inject destination anyways."
> +    fi
> +
> +    if [ ! -d "${inject_dest}" ]; then
> +	ptxd_bailout "<PKG> inject destination dir missing." \
> +	    "Correct placement of world/inject depends on <PKG>_BUILD_OOT and <PKG>_INJECT_OOT." \
> +	    "Check order of calls in prepare stage!"
>      fi
>  
>      for inject_file in ${pkg_inject_files}; do

Re: [ptxdist] [PATCH v2 0/4] ptxd_make_world_inject: Spring cleanup and optional dest dir

[Michael Riesch]

Hello Alexander,

On 5/6/24 12:50, Alexander Dahl wrote:
> Hello,
> 
> some spring cleaning and a new feature for the firmware inject
> mechanism.
> 
> When trying to re-build U-Boot as an oot build for the Karo QSBASE93
> Evalkit (for QS93 Solder-Down System-on-Module) it became apparent
> U-Boot wants some of those binary blobs in build folder instead of
> source folder.  Supporting this usecase is nice to have, because you
> won't clutter your source tree, especially when building from external
> tree after something like `ptxdist local-src u-boot ~/src/u-boot` …
> 
> Maybe we should add some documentation for this inject mechanism?
> 
> Follow-up patches for u-boot package in preparation already, but let me
> know what you think of this first.
> 
> Greets
> Alex
> 
> v2:
> - rework patch 4 introducing the new inject oot mechanism
>   (using the new <PKG>_INJECT_OOT variable now instead of a needlessly
>   flexible approach in v1)
> 
> v1: https://lore.ptxdist.org/ptxdist/20240424143109.277373-1-ada@thorsis.com/T/#t
> - initial patch stack revision (implicit)
> 
> Alexander Dahl (4):
>   ptxd_make_world_inject: Remove useless test
>   ptxd_make_world_inject: Use <PKG>_DIR directly
>   ptxd_make_world_inject: Escape inject path and files
>   ptxd_make_world_inject: Introduce new variable <PKG>_INJECT_OOT
> 
>  rules/post/ptxd_make_world_inject.make |  6 +++---
>  scripts/lib/ptxd_make_world_inject.sh  | 26 ++++++++++++++++++++++----
>  2 files changed, 25 insertions(+), 7 deletions(-)
> 
> 
> base-commit: 5a720c419cd1fabde04e8de8d1d1367c76a2666b

nice, thanks for your patches! Again, I am not at all an expert here and
only had a brief look over them, but they LGTM. FWIW:

Acked-by: Michael Riesch <michael.riesch@wolfvision.net>

Best regards,
Michael