[v1] UBOOT_SOURCE/SCRIPTS variables must always be set

[ImageBuilder] UBOOT_SOURCE/SCRIPTS variables must always be set

Posted by Luca Miccio 2 days, 20 hours ago

The current README incorrectly omits the need for the UBOOT_SOURCE and
UBOOT_SCRIPT variables for uboot-script-gen to function properly.
Moreover, the script silently fails if these variables are not set.
Return an error if the the UBOOT_SCRIPT variable is not set and
correct the documentation accordingly.

Signed-off-by: Luca Miccio <luca.miccio@amd.com>
---
 README.md                |  3 ++-
 scripts/uboot-script-gen | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 4fcd908..ae2fdfd 100644
--- a/README.md
+++ b/README.md
@@ -272,7 +272,8 @@ Where:
 
 - UBOOT_SOURCE and UBOOT_SCRIPT specify the output. They are optional
   as you can pass -o FILENAME to uboot-script-gen as a command line
-  parameter
+  parameter. It has to be set either in the config file or CLI argument
+  though.
 
 - APPEND_EXTRA_CMDS: is optional and specifies the path to a text file
   containing extra u-boot commands to be added to the boot script before
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index dcf5bdb..b07cbf1 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -1210,6 +1210,18 @@ then
     UBOOT_SCRIPT="$uboot_out_opt".scr
     UBOOT_SOURCE="$uboot_out_opt".source
 fi
+
+if test ! "$UBOOT_SOURCE"
+then
+    echo "UBOOT_SOURCE not set, either specify it in the config or set it with the -o option"
+    exit 1
+fi
+
+if test ! "$UBOOT_SCRIPT"
+then
+    UBOOT_SCRIPT="$UBOOT_SCRIPT".scr
+fi
+
 if test "$fit_opt" && ! test "$FIT"
 then
     FIT="${UBOOT_SOURCE%.source}.fit"
-- 
2.34.1

Re: [ImageBuilder] UBOOT_SOURCE/SCRIPTS variables must always be set

Posted by Stefano Stabellini 2 days, 9 hours ago

On Mon, 18 Nov 2024, Luca Miccio wrote:
> The current README incorrectly omits the need for the UBOOT_SOURCE and
> UBOOT_SCRIPT variables for uboot-script-gen to function properly.
> Moreover, the script silently fails if these variables are not set.
> Return an error if the the UBOOT_SCRIPT variable is not set and
> correct the documentation accordingly.
> 
> Signed-off-by: Luca Miccio <luca.miccio@amd.com>
> ---
>  README.md                |  3 ++-
>  scripts/uboot-script-gen | 12 ++++++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/README.md b/README.md
> index 4fcd908..ae2fdfd 100644
> --- a/README.md
> +++ b/README.md
> @@ -272,7 +272,8 @@ Where:
>  
>  - UBOOT_SOURCE and UBOOT_SCRIPT specify the output. They are optional
>    as you can pass -o FILENAME to uboot-script-gen as a command line
> -  parameter
> +  parameter. It has to be set either in the config file or CLI argument
> +  though.
>  
>  - APPEND_EXTRA_CMDS: is optional and specifies the path to a text file
>    containing extra u-boot commands to be added to the boot script before
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index dcf5bdb..b07cbf1 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -1210,6 +1210,18 @@ then
>      UBOOT_SCRIPT="$uboot_out_opt".scr
>      UBOOT_SOURCE="$uboot_out_opt".source
>  fi
> +
> +if test ! "$UBOOT_SOURCE"
> +then
> +    echo "UBOOT_SOURCE not set, either specify it in the config or set it with the -o option"
> +    exit 1
> +fi
> +
> +if test ! "$UBOOT_SCRIPT"
> +then
> +    UBOOT_SCRIPT="$UBOOT_SCRIPT".scr

This doesn't look right? Did you mean:

  UBOOT_SCRIPT="${UBOOT_SOURCE%.source}.scr"


> +fi
> +
>  if test "$fit_opt" && ! test "$FIT"
>  then
>      FIT="${UBOOT_SOURCE%.source}.fit"
> -- 
> 2.34.1
>

Re: [ImageBuilder] UBOOT_SOURCE/SCRIPTS variables must always be set

Posted by Miccio, Luca 2 days ago

Hi Stefano,

On 11/19/2024 1:23 AM, Stefano Stabellini wrote:
> 
> 
> On Mon, 18 Nov 2024, Luca Miccio wrote:
>> The current README incorrectly omits the need for the UBOOT_SOURCE and
>> UBOOT_SCRIPT variables for uboot-script-gen to function properly.
>> Moreover, the script silently fails if these variables are not set.
>> Return an error if the the UBOOT_SCRIPT variable is not set and
>> correct the documentation accordingly.
>>
>> Signed-off-by: Luca Miccio <luca.miccio@amd.com>
>> ---
>>  README.md                |  3 ++-
>>  scripts/uboot-script-gen | 12 ++++++++++++
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/README.md b/README.md
>> index 4fcd908..ae2fdfd 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -272,7 +272,8 @@ Where:
>>
>>  - UBOOT_SOURCE and UBOOT_SCRIPT specify the output. They are optional
>>    as you can pass -o FILENAME to uboot-script-gen as a command line
>> -  parameter
>> +  parameter. It has to be set either in the config file or CLI argument
>> +  though.
>>
>>  - APPEND_EXTRA_CMDS: is optional and specifies the path to a text file
>>    containing extra u-boot commands to be added to the boot script before
>> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
>> index dcf5bdb..b07cbf1 100755
>> --- a/scripts/uboot-script-gen
>> +++ b/scripts/uboot-script-gen
>> @@ -1210,6 +1210,18 @@ then
>>      UBOOT_SCRIPT="$uboot_out_opt".scr
>>      UBOOT_SOURCE="$uboot_out_opt".source
>>  fi
>> +
>> +if test ! "$UBOOT_SOURCE"
>> +then
>> +    echo "UBOOT_SOURCE not set, either specify it in the config or set it with the -o option"
>> +    exit 1
>> +fi
>> +
>> +if test ! "$UBOOT_SCRIPT"
>> +then
>> +    UBOOT_SCRIPT="$UBOOT_SCRIPT".scr
> 
> This doesn't look right? Did you mean:
> 
>   UBOOT_SCRIPT="${UBOOT_SOURCE%.source}.scr"
> 

Yes you're right. It's a typo. I will send a V2 right away.

> 
>> +fi
>> +
>>  if test "$fit_opt" && ! test "$FIT"
>>  then
>>      FIT="${UBOOT_SOURCE%.source}.fit"
>> --
>> 2.34.1
>>

Thanks,
Luca