diff mbox series

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

Message ID 20200812092705.17774-3-mhartmay@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series s390x: Add Protected VM support | expand

Commit Message

Marc Hartmayer Aug. 12, 2020, 9:27 a.m. UTC
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(-)

Comments

Andrew Jones Aug. 13, 2020, 7:49 a.m. UTC | #1
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, but
I'd prefer we make that dependency explicit. We could probably just
source it again from this file.

Thanks,
drew

> +if [ -f "${ARCH_FUNC}" ]; then
> +	source "${ARCH_FUNC}"
> +fi
> -- 
> 2.25.4
>
Marc Hartmayer Aug. 13, 2020, 11:45 a.m. UTC | #2
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?

>
> Thanks,
> drew
>
>> +if [ -f "${ARCH_FUNC}" ]; then
>> +	source "${ARCH_FUNC}"
>> +fi
>> -- 
>> 2.25.4
>> 
>
Andrew Jones Aug. 13, 2020, 12:07 p.m. UTC | #3
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.

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
>
Marc Hartmayer Aug. 13, 2020, 12:36 p.m. UTC | #4
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
>> 
>
diff mbox series

Patch

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
+if [ -f "${ARCH_FUNC}" ]; then
+	source "${ARCH_FUNC}"
+fi