Re: [Buildroot] [PATCH 3/4 v5] utils/test-pkg: add mode to only prepare .config files

[Romain Naour <romain.naour@gmail.com>]

Hello Arnout, Yann,

Le 05/08/2021 à 22:45, Arnout Vandecappelle a écrit :
> 
> 
> On 28/06/2021 22:15, Yann E. MORIN wrote:
>> Currently, running test-pkg is only done locally on the developpers
>> machine.
>>
>> In a follow up commit, we'll add the possibility to run test-pkg in a
>> gitlab-ci pipeline and, to speed up things, with one job per buildable
>> configuration.
>>
>> As such, we will need that test-pkg only ever prepares the
>> configuration, and that it does not build them.
>>
>> Add such a mode, with a new option, --prepare-only
>>
>> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
>> Cc: Romain Naour <romain.naour@gmail.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>
>> ---
>> Note: naming is hard; naming options is harder; naming options with a
>> terse term is even harder; naming options with a terse term that is
>> still meaningful and explains what the option does, is even harder yet.
> 
>  But doing it inconsistently is easy (see below). :-)
> 
>  Anyway, there's an easy solution to that (which I believe we should apply
> here): don't define a terse option.
> 
>  I think terse options should only be defined for stuff that a human has to
> type. In scripts, terse options shouldn't be used, because it makes it harder
> for the programmer to understand what the command does.
> 
>  Since prepare-only is meant ot be used by script, I don't think a terse option
> is needed.

Thanks for your advice. Indeed I don't think we need a terse option.

Best regards,
Romain

> 
> 
>>
>> ---
>> v5: split off from the next patch into this patch
>> ---
>>  utils/test-pkg | 22 ++++++++++++++++++----
>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/utils/test-pkg b/utils/test-pkg
>> index 54c6c5e8fe..4a20cab57f 100755
>> --- a/utils/test-pkg
>> +++ b/utils/test-pkg
>> @@ -12,13 +12,13 @@ do_clean() {
>>  
>>  main() {
>>      local o O opts
>> -    local cfg dir pkg random toolchains_csv toolchain all number mode
>> +    local cfg dir pkg random toolchains_csv toolchain all number mode prepare_only
>>      local ret nb nb_skip nb_fail nb_legal nb_tc build_dir keep
>>      local -a toolchains
>>      local pkg_br_name
>>  
>> -    o='hakc:d:n:p:r:t:'
>> -    O='help,all,keep,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:> +    o='hakgc:d:n:p:r:t:'
>              ^ AFAICS this is a 'g'...
> 
>> +    O='help,all,keep,prepare-only,config-snippet:,build-dir:,number:,package:,random:,toolchains-csv:'
>>      opts="$(getopt -n "${my_name}" -o "${o}" -l "${O}" -- "${@}")"
>>      eval set -- "${opts}"
>>  
>> @@ -27,6 +27,7 @@ main() {
>>      keep=0
>>      number=0
>>      mode=0
>> +    prepare_only=0
>>      toolchains_csv="${TOOLCHAINS_CSV}"
>>      while [ ${#} -gt 0 ]; do
>>          case "${1}" in
>> @@ -39,6 +40,9 @@ main() {
>>          (-k|--keep)
>>              keep=1; shift 1
>>              ;;
>> +        (-l|--prepare-only)
>              ^ ... but this is an 'l'!
> 
>  Clearly someone didn't test with the terse option. :-)
> 
>> +            prepare_only=1; shift 1
>> +            ;;
>>          (-c|--config-snippet)
>>              cfg="${2}"; shift 2
>>              ;;
>> @@ -127,7 +131,7 @@ main() {
>>          toolchain="$(basename "${toolchainconfig}" .config)"
>>          build_dir="${dir}/${toolchain}"
>>          printf "%40s [%*d/%d]: " "${toolchain}" ${#nb_tc} ${nb} ${nb_tc}
>> -        build_one "${build_dir}" "${toolchainconfig}" "${cfg}" "${pkg}" && ret=0 || ret=${?}
>> +        build_one "${build_dir}" "${toolchainconfig}" "${cfg}" "${pkg}" "${prepare_only}" && ret=0 || ret=${?}
>>          case ${ret} in
>>          (0) printf "OK\n";;
>>          (1) : $((nb_skip++)); printf "SKIPPED\n";;
>> @@ -147,6 +151,7 @@ build_one() {
>>      local toolchainconfig="${2}"
>>      local cfg="${3}"
>>      local pkg="${4}"
>> +    local defer="${5}"
> 
>  I like it if variables that are the same are called the same. I.e., use
> prepare_only here as well. Perhaps you can even skip passing it at all since
> it's a global variable.
> 
>>  
>>      mkdir -p "${dir}"
>>  
>> @@ -170,6 +175,11 @@ build_one() {
>>      # Remove file, it's empty anyway.
>>      rm -f "${dir}/missing.config"
>>  
>> +    # Defer building the job to the caller (e.g. a gitlab pipeline)
>> +    if [ ${defer} -eq 1 ]; then
>> +        return 0
>> +    fi
>> +
>>      if [ -n "${pkg}" ]; then
>>          if ! make O="${dir}" "${pkg}-dirclean" >> "${dir}/logfile" 2>&1; then
>>              return 2
>> @@ -257,6 +267,10 @@ Options:
>>          Note: the logfile and configuration is always retained, even without
>>          this option.
>>  
>> +    --prepare-only
> 
>  And the terse option isn't even in the help! So nobody would use it anyway...
> 
> 
>  Regards,
>  Arnout
> 
>> +        Only prepare the .config files, but do not build them. Output the
>> +        list of build directories to stdout, and the status on stderr.
>> +
>>  Example:
>>  
>>      Testing libcec would require a config snippet that contains:
>>

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot