Re: [kvm-unit-tests RFC v2 2/4] scripts: add support for architecture dependent functions

[Marc Hartmayer <mhartmay@linux.ibm.com>]

On Thu, Aug 13, 2020 at 02:07 PM +0200, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Aug 13, 2020 at 01:45:46PM +0200, Marc Hartmayer wrote:
>> On Thu, Aug 13, 2020 at 09:49 AM +0200, Andrew Jones <drjones@redhat.com> wrote:
>> > On Wed, Aug 12, 2020 at 11:27:03AM +0200, Marc Hartmayer wrote:
>> >> This is necessary to keep architecture dependent code separate from
>> >> common code.
>> >> 
>> >> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>> >> ---
>> >>  README.md           | 3 ++-
>> >>  scripts/common.bash | 5 +++++
>> >>  2 files changed, 7 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/README.md b/README.md
>> >> index 48be206c6db1..24d4bdaaee0d 100644
>> >> --- a/README.md
>> >> +++ b/README.md
>> >> @@ -134,7 +134,8 @@ all unit tests.
>> >>  ## Directory structure
>> >>  
>> >>      .:                  configure script, top-level Makefile, and run_tests.sh
>> >> -    ./scripts:          helper scripts for building and running tests
>> >> +    ./scripts:          general architecture neutral helper scripts for building and running tests
>> >> +    ./scripts/<ARCH>:   architecture dependent helper scripts for building and running tests
>> >>      ./lib:              general architecture neutral services for the tests
>> >>      ./lib/<ARCH>:       architecture dependent services for the tests
>> >>      ./<ARCH>:           the sources of the tests and the created objects/images
>> >> diff --git a/scripts/common.bash b/scripts/common.bash
>> >> index 96655c9ffd1f..f9c15fd304bd 100644
>> >> --- a/scripts/common.bash
>> >> +++ b/scripts/common.bash
>> >> @@ -52,3 +52,8 @@ function for_each_unittest()
>> >>  	fi
>> >>  	exec {fd}<&-
>> >>  }
>> >> +
>> >> +ARCH_FUNC=scripts/${ARCH}/func.bash
>> >
>> > The use of ${ARCH} adds a dependency on config.mak. It works now because
>> > in the two places we source common.bash we source config.mak first
>> 
>> Yep, I know.
>> 
>> > , but
>> > I'd prefer we make that dependency explicit.
>> 
>> Okay.
>> 
>> > We could probably just
>> > source it again from this file.
>> 
>> Another option is to pass ${ARCH} as an argument when we `source
>> scripts/runtime.bash`
>> 
>> => `source scripts/runtime.bash "${ARCH}"`
>> 
>> Which one do you prefer?
>
> The first one. There's a chance that the arch helper functions will
> need more than $ARCH from config.mak. Of course that means we have
> a dependency on config.mak from the arch helper file too. We can
> just add a comment in common.bash about the order of sourcing
> though, as common.bash should be the only file sourcing the
> arch helper file.

Will add it. Thanks!

>
> Thanks,
> drew
>
>> 
>> >
>> > Thanks,
>> > drew
>> >
>> >> +if [ -f "${ARCH_FUNC}" ]; then
>> >> +	source "${ARCH_FUNC}"
>> >> +fi
>> >> -- 
>> >> 2.25.4
>> >> 
>> >
>> -- 
>> Kind regards / Beste Grüße
>>    Marc Hartmayer
>> 
>> IBM Deutschland Research & Development GmbH
>> Vorsitzender des Aufsichtsrats: Gregor Pillen 
>> Geschäftsführung: Dirk Wittkopp
>> Sitz der Gesellschaft: Böblingen
>> Registergericht: Amtsgericht Stuttgart, HRB 243294
>> 
>
-- 
Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294