Re: [PATCH] build: Remove enabled-by special case

2023-09-18 Thread Chris Johns
Looks good.

Thanks
Chris

On 18/9/2023 8:26 pm, Sebastian Huber wrote:
> Doing the enabled-by processing just for the ldflags and just for the
> link custom commands is confusing.  Use an option instead which is
> intended to be used for such use cases.
> ---
>  spec/build/testsuites/libtests/dl07.yml   |  4 +---
>  spec/build/testsuites/libtests/dl08.yml   |  4 +---
>  spec/build/testsuites/libtests/dl09.yml   |  4 +---
>  spec/build/testsuites/libtests/grp.yml|  2 ++
>  .../testsuites/libtests/optdlldflags.yml  | 22 +++
>  wscript   | 12 +++---
>  6 files changed, 30 insertions(+), 18 deletions(-)
>  create mode 100644 spec/build/testsuites/libtests/optdlldflags.yml
> 
> diff --git a/spec/build/testsuites/libtests/dl07.yml 
> b/spec/build/testsuites/libtests/dl07.yml
> index e443f4ed29..5760f68b87 100644
> --- a/spec/build/testsuites/libtests/dl07.yml
> +++ b/spec/build/testsuites/libtests/dl07.yml
> @@ -33,9 +33,7 @@ enabled-by:
>  includes:
>  - testsuites/libtests/dl07
>  ldflags:
> -- enabled-by:
> -  - microblaze
> -  value: -u__extendsfdf2
> +- ${LIBDL_TESTS_LDFLAGS}
>  links: []
>  prepare-build: null
>  prepare-configure: null
> diff --git a/spec/build/testsuites/libtests/dl08.yml 
> b/spec/build/testsuites/libtests/dl08.yml
> index 8e5eec77f2..ada6caf698 100644
> --- a/spec/build/testsuites/libtests/dl08.yml
> +++ b/spec/build/testsuites/libtests/dl08.yml
> @@ -38,9 +38,7 @@ enabled-by:
>  includes:
>  - testsuites/libtests/dl08
>  ldflags:
> -- enabled-by:
> -  - microblaze
> -  value: -u__extendsfdf2
> +- ${LIBDL_TESTS_LDFLAGS}
>  links: []
>  prepare-build: null
>  prepare-configure: null
> diff --git a/spec/build/testsuites/libtests/dl09.yml 
> b/spec/build/testsuites/libtests/dl09.yml
> index 2d00286c15..ee0be57fb2 100644
> --- a/spec/build/testsuites/libtests/dl09.yml
> +++ b/spec/build/testsuites/libtests/dl09.yml
> @@ -33,9 +33,7 @@ enabled-by:
>  includes:
>  - testsuites/libtests/dl09
>  ldflags:
> -- enabled-by:
> -  - microblaze
> -  value: -u__extendsfdf2
> +- ${LIBDL_TESTS_LDFLAGS}
>  links: []
>  prepare-build: null
>  prepare-configure: null
> diff --git a/spec/build/testsuites/libtests/grp.yml 
> b/spec/build/testsuites/libtests/grp.yml
> index c1a6209e99..eaf21751c4 100644
> --- a/spec/build/testsuites/libtests/grp.yml
> +++ b/spec/build/testsuites/libtests/grp.yml
> @@ -22,6 +22,8 @@ links:
>uid: ../optgzip
>  - role: build-dependency
>uid: ../optxz
> +- role: build-dependency
> +  uid: optdlldflags
>  - role: build-dependency
>uid: optrtemsld
>  - role: build-dependency
> diff --git a/spec/build/testsuites/libtests/optdlldflags.yml 
> b/spec/build/testsuites/libtests/optdlldflags.yml
> new file mode 100644
> index 00..0d754c5270
> --- /dev/null
> +++ b/spec/build/testsuites/libtests/optdlldflags.yml
> @@ -0,0 +1,22 @@
> +SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
> +actions:
> +- get-string: null
> +- split: null
> +- env-assign: null
> +build-type: option
> +copyrights:
> +- Copyright (C) 2023 embedded brains GmbH & Co. KG
> +default:
> +- enabled-by: microblaze
> +  value:
> +  - -u__extendsfdf2
> +- enabled-by: true
> +  value: []
> +description: |
> +  Linker flags used to link libdl tests with a base image.  For example, it 
> may
> +  be used to add undefined symbols which the linker has to resolve to pull in
> +  services required by loaded parts.
> +enabled-by: true
> +links: []
> +name: LIBDL_TESTS_LDFLAGS
> +type: build
> diff --git a/wscript b/wscript
> index 00b81b4874..65f90fc324 100755
> --- a/wscript
> +++ b/wscript
> @@ -360,7 +360,7 @@ class Item(object):
>  def __init__(self, item, bic, cmd, env, ldflags):
>  super(link, self).__init__(self, env=env)
>  self.cmd = cmd
> -self.ldflags = bic.ldflags + ldflags
> +self.ldflags = ldflags
>  self.stlib = item.data["stlib"]
>  self.use = (item.data["use-before"] + bic.use +
>  item.data["use-after"])
> @@ -386,14 +386,8 @@ class Item(object):
>  [],
>  )
>  
> -ldflags = []
> -for ldflag in self.data["ldflags"]:
> -if isinstance(ldflag, dict):
> -if _is_enabled(bld.env.ENABLE, ldflag["enabled-by"]):
> -ldflags.append(ldflag["value"])
> -else:
> -ldflags.append(ldflag)
> -tsk = link(self, bic, cmd, bld.env, ldflags)
> +tsk = link(self, bic, cmd, bld.env,
> +   bic.ldflags + self.substitute(bld, self.data["ldflags"]))
>  tsk.set_inputs([bld.bldnode.make_node(s) for s in source])
>  tsk.set_outputs(bld.bldnode.make_node(target))
>  bld.add_to_group(tsk)
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [rtems commit] libdl: Add support to import base image TLS symbols

2023-09-18 Thread Chris Johns
On 18/9/2023 10:09 pm, Sebastian Huber wrote:
> On 17.09.23 00:58, Chris Johns wrote:
>>> There are also no test cases for this function.
>>> Without test cases it is easy to break something without knowing it.
>> The libdl test cases have been updated to reference TLS newlib variables to 
>> test
>> this. Is that enough? If not what would you like to see?
> 
> These higher level tests are all right, however, it would be nice to have also
> lower level tests which just check a particular function like
> rtems_rtl_tls_get_base(). This would improve the maintainability of this 
> stuff.
> You would not have to debug a tool generated code to figure out what is wrong.

This makes sense. I will think about a suitable test.

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


Re: [rtems commit] libdl: Add support to import base image TLS symbols

2023-09-18 Thread Sebastian Huber

On 17.09.23 00:58, Chris Johns wrote:

There are also no test cases for this function.
Without test cases it is easy to break something without knowing it.

The libdl test cases have been updated to reference TLS newlib variables to test
this. Is that enough? If not what would you like to see?


These higher level tests are all right, however, it would be nice to 
have also lower level tests which just check a particular function like 
rtems_rtl_tls_get_base(). This would improve the maintainability of this 
stuff. You would not have to debug a tool generated code to figure out 
what is wrong.


--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH] build: Remove enabled-by special case

2023-09-18 Thread Sebastian Huber
Doing the enabled-by processing just for the ldflags and just for the
link custom commands is confusing.  Use an option instead which is
intended to be used for such use cases.
---
 spec/build/testsuites/libtests/dl07.yml   |  4 +---
 spec/build/testsuites/libtests/dl08.yml   |  4 +---
 spec/build/testsuites/libtests/dl09.yml   |  4 +---
 spec/build/testsuites/libtests/grp.yml|  2 ++
 .../testsuites/libtests/optdlldflags.yml  | 22 +++
 wscript   | 12 +++---
 6 files changed, 30 insertions(+), 18 deletions(-)
 create mode 100644 spec/build/testsuites/libtests/optdlldflags.yml

diff --git a/spec/build/testsuites/libtests/dl07.yml 
b/spec/build/testsuites/libtests/dl07.yml
index e443f4ed29..5760f68b87 100644
--- a/spec/build/testsuites/libtests/dl07.yml
+++ b/spec/build/testsuites/libtests/dl07.yml
@@ -33,9 +33,7 @@ enabled-by:
 includes:
 - testsuites/libtests/dl07
 ldflags:
-- enabled-by:
-  - microblaze
-  value: -u__extendsfdf2
+- ${LIBDL_TESTS_LDFLAGS}
 links: []
 prepare-build: null
 prepare-configure: null
diff --git a/spec/build/testsuites/libtests/dl08.yml 
b/spec/build/testsuites/libtests/dl08.yml
index 8e5eec77f2..ada6caf698 100644
--- a/spec/build/testsuites/libtests/dl08.yml
+++ b/spec/build/testsuites/libtests/dl08.yml
@@ -38,9 +38,7 @@ enabled-by:
 includes:
 - testsuites/libtests/dl08
 ldflags:
-- enabled-by:
-  - microblaze
-  value: -u__extendsfdf2
+- ${LIBDL_TESTS_LDFLAGS}
 links: []
 prepare-build: null
 prepare-configure: null
diff --git a/spec/build/testsuites/libtests/dl09.yml 
b/spec/build/testsuites/libtests/dl09.yml
index 2d00286c15..ee0be57fb2 100644
--- a/spec/build/testsuites/libtests/dl09.yml
+++ b/spec/build/testsuites/libtests/dl09.yml
@@ -33,9 +33,7 @@ enabled-by:
 includes:
 - testsuites/libtests/dl09
 ldflags:
-- enabled-by:
-  - microblaze
-  value: -u__extendsfdf2
+- ${LIBDL_TESTS_LDFLAGS}
 links: []
 prepare-build: null
 prepare-configure: null
diff --git a/spec/build/testsuites/libtests/grp.yml 
b/spec/build/testsuites/libtests/grp.yml
index c1a6209e99..eaf21751c4 100644
--- a/spec/build/testsuites/libtests/grp.yml
+++ b/spec/build/testsuites/libtests/grp.yml
@@ -22,6 +22,8 @@ links:
   uid: ../optgzip
 - role: build-dependency
   uid: ../optxz
+- role: build-dependency
+  uid: optdlldflags
 - role: build-dependency
   uid: optrtemsld
 - role: build-dependency
diff --git a/spec/build/testsuites/libtests/optdlldflags.yml 
b/spec/build/testsuites/libtests/optdlldflags.yml
new file mode 100644
index 00..0d754c5270
--- /dev/null
+++ b/spec/build/testsuites/libtests/optdlldflags.yml
@@ -0,0 +1,22 @@
+SPDX-License-Identifier: CC-BY-SA-4.0 OR BSD-2-Clause
+actions:
+- get-string: null
+- split: null
+- env-assign: null
+build-type: option
+copyrights:
+- Copyright (C) 2023 embedded brains GmbH & Co. KG
+default:
+- enabled-by: microblaze
+  value:
+  - -u__extendsfdf2
+- enabled-by: true
+  value: []
+description: |
+  Linker flags used to link libdl tests with a base image.  For example, it may
+  be used to add undefined symbols which the linker has to resolve to pull in
+  services required by loaded parts.
+enabled-by: true
+links: []
+name: LIBDL_TESTS_LDFLAGS
+type: build
diff --git a/wscript b/wscript
index 00b81b4874..65f90fc324 100755
--- a/wscript
+++ b/wscript
@@ -360,7 +360,7 @@ class Item(object):
 def __init__(self, item, bic, cmd, env, ldflags):
 super(link, self).__init__(self, env=env)
 self.cmd = cmd
-self.ldflags = bic.ldflags + ldflags
+self.ldflags = ldflags
 self.stlib = item.data["stlib"]
 self.use = (item.data["use-before"] + bic.use +
 item.data["use-after"])
@@ -386,14 +386,8 @@ class Item(object):
 [],
 )
 
-ldflags = []
-for ldflag in self.data["ldflags"]:
-if isinstance(ldflag, dict):
-if _is_enabled(bld.env.ENABLE, ldflag["enabled-by"]):
-ldflags.append(ldflag["value"])
-else:
-ldflags.append(ldflag)
-tsk = link(self, bic, cmd, bld.env, ldflags)
+tsk = link(self, bic, cmd, bld.env,
+   bic.ldflags + self.substitute(bld, self.data["ldflags"]))
 tsk.set_inputs([bld.bldnode.make_node(s) for s in source])
 tsk.set_outputs(bld.bldnode.make_node(target))
 bld.add_to_group(tsk)
-- 
2.35.3

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel