Re: [PATCH] wscript: Deduplicate installed files

2023-02-06 Thread Gedare Bloom 
On Mon, Feb 6, 2023 at 1:03 PM Kinsey Moore  wrote:
>
> On 2/6/2023 8:44 AM, Sebastian Huber wrote:
> > On 06.02.23 15:30, Joel Sherrill wrote:
> >> On Mon, Feb 6, 2023 at 12:55 AM Sebastian Huber
> >>  >> > wrote:
> >> On 06.02.23 03:35, Chris Johns wrote:
> >>  > On 4/2/2023 6:11 am, Sebastian Huber wrote:
> >>  >> On 03.02.23 19:45, Kinsey Moore wrote:
> >>  >>> This is my first stab at solving this duplicate install
> >> problem. I could
> >>  >>> manually solve the problem by deduplicating the object includes
> >> and moving it
> >>  >>> up to the BSP, but that is less intuitive since these drivers
> >> both depend on
> >>  >>> the same code and the BSP doesn't depend on it directly.
> >>  >>
> >>  >> Why don't you add the shared stuff to a objxilcommon.yml?
> >>  >>
> >>  >> The approach in the wscript is a bit complex from my point of
> >> view.
> >>  >
> >>  > I am OK with adding this code or something similar. It is no more
> >> complex than
> >>  > other places I have reviewed, eg `Item._init_link()`.
> >>  >
> >>  > The issue is currently not easy to see and may be present in
> >> other places
> >>  > without us knowing. I am also fine with a spec file check that
> >> highlights a
> >>  > clash to draw attention to a problem when the spec files are
> >> parsed. I feeling
> >>  > we need something.
> >>
> >> If you install with
> >>
> >> ./waf install -vv
> >>
> >> you see the duplicate install targets. See also
> >>
> >> https://gitlab.com/ita1024/waf/-/issues/2329#note_467849523
> >> 
> >>
> >> Before we add double for loops we should first analyze the
> >> underlying
> >> problem. In this case it is a diamond shaped build dependency graph.
> >>
> >> spec/build/bsps/objnandpsu.yml:  uid: objxilinxsupport
> >> spec/build/bsps/objqspipsu.yml:  uid: objxilinxsupport
> >> spec/build/bsps/aarch64/xilinx-zynqmp/objjffs2qspinor.yml: uid:
> >> ../../objqspipsu
> >> spec/build/bsps/aarch64/xilinx-zynqmp/grp_zu3eg.yml:  uid:
> >> ../../objnandpsu
> >>
> >> In addition to the duplicate install targets you build also the
> >> objects
> >> of objxilinxsupport twice and add them to the library.
> >>
> >> I would simply move the links to grp_zu3eg:
> >>
> >> grp_zu3eg.yml:  uid: ../../objxilinxspport
> >> grp_zu3eg.yml:  uid: ../../objnandpsu
> >> grp_zu3eg.yml:  uid: ../../objqspipsu
> >>
> >>
> >> That's the solution in this case but wouldn't it be nice to detect it
> >> reliably and
> >> address it when it shows up again.
> >
> > You can detect the issue with by calling ./waf -vv install. This is
> > what the maintainer of waf recommends:
> >
> > https://gitlab.com/ita1024/waf/-/issues/2329#note_467849523
> >
> > I would rather try to get these warnings by default from waf instead
> > of tinkering with special case solutions above waf.
> >
> > The patch doesn't address the issue with the multiple object files in
> > the library.
> >
> It sounds like we consider this an error which is fine by me. I'll drop
> this patch and deduplicate the objects manually.
>
> I would like to find a way to make this an actual error which will stop
> the build instead of hiding a warning behind the -v flag. I'll see what
> I can do along these lines.
>
The linked Issue suggests that it occasionally comes up as a false
positive, which is why tnagy didn't make it an error.

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

Re: [PATCH] wscript: Deduplicate installed files

2023-02-06 Thread Kinsey Moore 

On 2/6/2023 8:44 AM, Sebastian Huber wrote:

On 06.02.23 15:30, Joel Sherrill wrote:
On Mon, Feb 6, 2023 at 12:55 AM Sebastian Huber 
> wrote:

    On 06.02.23 03:35, Chris Johns wrote:
 > On 4/2/2023 6:11 am, Sebastian Huber wrote:
 >> On 03.02.23 19:45, Kinsey Moore wrote:
 >>> This is my first stab at solving this duplicate install
    problem. I could
 >>> manually solve the problem by deduplicating the object includes
    and moving it
 >>> up to the BSP, but that is less intuitive since these drivers
    both depend on
 >>> the same code and the BSP doesn't depend on it directly.
 >>
 >> Why don't you add the shared stuff to a objxilcommon.yml?
 >>
 >> The approach in the wscript is a bit complex from my point of 
view.

 >
 > I am OK with adding this code or something similar. It is no more
    complex than
 > other places I have reviewed, eg `Item._init_link()`.
 >
 > The issue is currently not easy to see and may be present in
    other places
 > without us knowing. I am also fine with a spec file check that
    highlights a
 > clash to draw attention to a problem when the spec files are
    parsed. I feeling
 > we need something.

    If you install with

    ./waf install -vv

    you see the duplicate install targets. See also

    https://gitlab.com/ita1024/waf/-/issues/2329#note_467849523


    Before we add double for loops we should first analyze the 
underlying

    problem. In this case it is a diamond shaped build dependency graph.

    spec/build/bsps/objnandpsu.yml:  uid: objxilinxsupport
    spec/build/bsps/objqspipsu.yml:  uid: objxilinxsupport
    spec/build/bsps/aarch64/xilinx-zynqmp/objjffs2qspinor.yml: uid:
    ../../objqspipsu
    spec/build/bsps/aarch64/xilinx-zynqmp/grp_zu3eg.yml:  uid:
    ../../objnandpsu

    In addition to the duplicate install targets you build also the 
objects

    of objxilinxsupport twice and add them to the library.

    I would simply move the links to grp_zu3eg:

    grp_zu3eg.yml:  uid: ../../objxilinxspport
    grp_zu3eg.yml:  uid: ../../objnandpsu
    grp_zu3eg.yml:  uid: ../../objqspipsu


That's the solution in this case but wouldn't it be nice to detect it 
reliably and

address it when it shows up again.


You can detect the issue with by calling ./waf -vv install. This is 
what the maintainer of waf recommends:


https://gitlab.com/ita1024/waf/-/issues/2329#note_467849523

I would rather try to get these warnings by default from waf instead 
of tinkering with special case solutions above waf.


The patch doesn't address the issue with the multiple object files in 
the library.


It sounds like we consider this an error which is fine by me. I'll drop 
this patch and deduplicate the objects manually.


I would like to find a way to make this an actual error which will stop 
the build instead of hiding a warning behind the -v flag. I'll see what 
I can do along these lines.



Kinsey

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

Re: [PATCH] wscript: Deduplicate installed files

2023-02-06 Thread Sebastian Huber 

On 06.02.23 15:30, Joel Sherrill wrote:
On Mon, Feb 6, 2023 at 12:55 AM Sebastian Huber 
> wrote:




On 06.02.23 03:35, Chris Johns wrote:
 > On 4/2/2023 6:11 am, Sebastian Huber wrote:
 >> On 03.02.23 19:45, Kinsey Moore wrote:
 >>> This is my first stab at solving this duplicate install
problem. I could
 >>> manually solve the problem by deduplicating the object includes
and moving it
 >>> up to the BSP, but that is less intuitive since these drivers
both depend on
 >>> the same code and the BSP doesn't depend on it directly.
 >>
 >> Why don't you add the shared stuff to a objxilcommon.yml?
 >>
 >> The approach in the wscript is a bit complex from my point of view.
 >
 > I am OK with adding this code or something similar. It is no more
complex than
 > other places I have reviewed, eg `Item._init_link()`.
 >
 > The issue is currently not easy to see and may be present in
other places
 > without us knowing. I am also fine with a spec file check that
highlights a
 > clash to draw attention to a problem when the spec files are
parsed. I feeling
 > we need something.

If you install with

./waf install -vv

you see the duplicate install targets. See also

https://gitlab.com/ita1024/waf/-/issues/2329#note_467849523


Before we add double for loops we should first analyze the underlying
problem. In this case it is a diamond shaped build dependency graph.

spec/build/bsps/objnandpsu.yml:  uid: objxilinxsupport
spec/build/bsps/objqspipsu.yml:  uid: objxilinxsupport
spec/build/bsps/aarch64/xilinx-zynqmp/objjffs2qspinor.yml:  uid:
../../objqspipsu
spec/build/bsps/aarch64/xilinx-zynqmp/grp_zu3eg.yml:  uid:
../../objnandpsu

In addition to the duplicate install targets you build also the objects
of objxilinxsupport twice and add them to the library.

I would simply move the links to grp_zu3eg:

grp_zu3eg.yml:  uid: ../../objxilinxspport
grp_zu3eg.yml:  uid: ../../objnandpsu
grp_zu3eg.yml:  uid: ../../objqspipsu


That's the solution in this case but wouldn't it be nice to detect it 
reliably and

address it when it shows up again.


You can detect the issue with by calling ./waf -vv install. This is what 
the maintainer of waf recommends:


https://gitlab.com/ita1024/waf/-/issues/2329#note_467849523

I would rather try to get these warnings by default from waf instead of 
tinkering with special case solutions above waf.


The patch doesn't address the issue with the multiple object files in 
the library.


--
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

Re: [PATCH] wscript: Deduplicate installed files

2023-02-06 Thread Joel Sherrill 
On Mon, Feb 6, 2023 at 12:55 AM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

>
>
> On 06.02.23 03:35, Chris Johns wrote:
> > On 4/2/2023 6:11 am, Sebastian Huber wrote:
> >> On 03.02.23 19:45, Kinsey Moore wrote:
> >>> This is my first stab at solving this duplicate install problem. I
> could
> >>> manually solve the problem by deduplicating the object includes and
> moving it
> >>> up to the BSP, but that is less intuitive since these drivers both
> depend on
> >>> the same code and the BSP doesn't depend on it directly.
> >>
> >> Why don't you add the shared stuff to a objxilcommon.yml?
> >>
> >> The approach in the wscript is a bit complex from my point of view.
> >
> > I am OK with adding this code or something similar. It is no more
> complex than
> > other places I have reviewed, eg `Item._init_link()`.
> >
> > The issue is currently not easy to see and may be present in other places
> > without us knowing. I am also fine with a spec file check that
> highlights a
> > clash to draw attention to a problem when the spec files are parsed. I
> feeling
> > we need something.
>
> If you install with
>
> ./waf install -vv
>
> you see the duplicate install targets. See also
>
> https://gitlab.com/ita1024/waf/-/issues/2329#note_467849523
>
> Before we add double for loops we should first analyze the underlying
> problem. In this case it is a diamond shaped build dependency graph.
>
> spec/build/bsps/objnandpsu.yml:  uid: objxilinxsupport
> spec/build/bsps/objqspipsu.yml:  uid: objxilinxsupport
> spec/build/bsps/aarch64/xilinx-zynqmp/objjffs2qspinor.yml:  uid:
> ../../objqspipsu
> spec/build/bsps/aarch64/xilinx-zynqmp/grp_zu3eg.yml:  uid: ../../objnandpsu
>
> In addition to the duplicate install targets you build also the objects
> of objxilinxsupport twice and add them to the library.
>
> I would simply move the links to grp_zu3eg:
>
> grp_zu3eg.yml:  uid: ../../objxilinxspport
> grp_zu3eg.yml:  uid: ../../objnandpsu
> grp_zu3eg.yml:  uid: ../../objqspipsu
>

That's the solution in this case but wouldn't it be nice to detect it
reliably and
address it when it shows up again.

--joel

>
> --
> 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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] wscript: Deduplicate installed files

2023-02-05 Thread Sebastian Huber 



On 06.02.23 03:35, Chris Johns wrote:

On 4/2/2023 6:11 am, Sebastian Huber wrote:

On 03.02.23 19:45, Kinsey Moore wrote:

This is my first stab at solving this duplicate install problem. I could
manually solve the problem by deduplicating the object includes and moving it
up to the BSP, but that is less intuitive since these drivers both depend on
the same code and the BSP doesn't depend on it directly.


Why don't you add the shared stuff to a objxilcommon.yml?

The approach in the wscript is a bit complex from my point of view.


I am OK with adding this code or something similar. It is no more complex than
other places I have reviewed, eg `Item._init_link()`.

The issue is currently not easy to see and may be present in other places
without us knowing. I am also fine with a spec file check that highlights a
clash to draw attention to a problem when the spec files are parsed. I feeling
we need something.


If you install with

./waf install -vv

you see the duplicate install targets. See also

https://gitlab.com/ita1024/waf/-/issues/2329#note_467849523

Before we add double for loops we should first analyze the underlying 
problem. In this case it is a diamond shaped build dependency graph.


spec/build/bsps/objnandpsu.yml:  uid: objxilinxsupport
spec/build/bsps/objqspipsu.yml:  uid: objxilinxsupport
spec/build/bsps/aarch64/xilinx-zynqmp/objjffs2qspinor.yml:  uid: 
../../objqspipsu

spec/build/bsps/aarch64/xilinx-zynqmp/grp_zu3eg.yml:  uid: ../../objnandpsu

In addition to the duplicate install targets you build also the objects 
of objxilinxsupport twice and add them to the library.


I would simply move the links to grp_zu3eg:

grp_zu3eg.yml:  uid: ../../objxilinxspport
grp_zu3eg.yml:  uid: ../../objnandpsu
grp_zu3eg.yml:  uid: ../../objqspipsu

--
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

Re: [PATCH] wscript: Deduplicate installed files

2023-02-05 Thread Chris Johns 
On 4/2/2023 6:11 am, Sebastian Huber wrote:
> On 03.02.23 19:45, Kinsey Moore wrote:
>> This is my first stab at solving this duplicate install problem. I could
>> manually solve the problem by deduplicating the object includes and moving it
>> up to the BSP, but that is less intuitive since these drivers both depend on
>> the same code and the BSP doesn't depend on it directly.
> 
> Why don't you add the shared stuff to a objxilcommon.yml?
> 
> The approach in the wscript is a bit complex from my point of view.

I am OK with adding this code or something similar. It is no more complex than
other places I have reviewed, eg `Item._init_link()`.

The issue is currently not easy to see and may be present in other places
without us knowing. I am also fine with a spec file check that highlights a
clash to draw attention to a problem when the spec files are parsed. I feeling
we need something.

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

Re: [PATCH] wscript: Deduplicate installed files

2023-02-03 Thread Kinsey Moore 
It's already in a common objxilinxsupport.yml file. The problem is that 
it is being included by two different drivers both imported from the 
xilinx upstream driver repo, so it currently attempts to install those 
shared headers twice. I suppose the solution could be to bundle the 
shared code together with any drivers that depend on it in a single 
obj*.yml.



Kinsey

On 2/3/2023 1:11 PM, Sebastian Huber wrote:

On 03.02.23 19:45, Kinsey Moore wrote:
This is my first stab at solving this duplicate install problem. I 
could manually solve the problem by deduplicating the object includes 
and moving it up to the BSP, but that is less intuitive since these 
drivers both depend on the same code and the BSP doesn't depend on it 
directly.


Why don't you add the shared stuff to a objxilcommon.yml?

The approach in the wscript is a bit complex from my point of view.


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

Re: [PATCH] wscript: Deduplicate installed files

2023-02-03 Thread Sebastian Huber 

On 03.02.23 19:45, Kinsey Moore wrote:
This is my first stab at solving this duplicate install problem. I could 
manually solve the problem by deduplicating the object includes and 
moving it up to the BSP, but that is less intuitive since these drivers 
both depend on the same code and the BSP doesn't depend on it directly.


Why don't you add the shared stuff to a objxilcommon.yml?

The approach in the wscript is a bit complex from my point of view.

--
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

Re: [PATCH] wscript: Deduplicate installed files

2023-02-03 Thread Kinsey Moore 
This is my first stab at solving this duplicate install problem. I could 
manually solve the problem by deduplicating the object includes and 
moving it up to the BSP, but that is less intuitive since these drivers 
both depend on the same code and the BSP doesn't depend on it directly.



Kinsey

On 2/3/2023 12:36 PM, Kinsey Moore wrote:

The addition of the NAND and NOR drivers both depending on the Xilinx
support code independently has introduced the possibility of duplicate
installed headers. This duplication results in multiple header install
attempts which can conflict since the installs can run on multiple CPUs.
This change to wscript deduplicates the header installs individually as
necessary. This change ignores identical installs and throws an error on
different header installs to the same location.
---
  wscript | 33 -
  1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/wscript b/wscript
index a34cac51e2..385ba8eb1c 100755
--- a/wscript
+++ b/wscript
@@ -269,8 +269,39 @@ class Item(object):
  bld.install_files(install_path, self.get(bld, "target"))
  
  def install_files(self, bld):

+if "rtems_installed" not in bld.env:
+bld.env["rtems_installed"] = {}
+
  for install in self.data["install"]:
-bld.install_files(install["destination"], install["source"])
+# setup destination array if it doesn't exist
+dest = install["destination"]
+if dest not in bld.env.rtems_installed:
+bld.env["rtems_installed"][dest] = []
+
+# build deduplicated install set
+dedup_set = []
+
+for item in install["source"]:
+# search for duplicate installs
+match_found = False
+filename = os.path.basename(item)
+
+for existing in bld.env["rtems_installed"][dest]:
+if existing[0] == filename:
+# duplicate found
+if item != existing[1]:
+bld.fatal(("File installs {} and {} " +
+"target the same location").format(
+item, existing[1]))
+match_found = True
+break
+
+if not match_found:
+dedup_set.append(item)
+bld.env["rtems_installed"][dest].append([filename, item])
+
+if len(dedup_set):
+bld.install_files(dest, dedup_set)
  
  def asm(self, bld, bic, source, target=None):

  if target is None:

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

[PATCH] wscript: Deduplicate installed files

2023-02-03 Thread Kinsey Moore 
The addition of the NAND and NOR drivers both depending on the Xilinx
support code independently has introduced the possibility of duplicate
installed headers. This duplication results in multiple header install
attempts which can conflict since the installs can run on multiple CPUs.
This change to wscript deduplicates the header installs individually as
necessary. This change ignores identical installs and throws an error on
different header installs to the same location.
---
 wscript | 33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/wscript b/wscript
index a34cac51e2..385ba8eb1c 100755
--- a/wscript
+++ b/wscript
@@ -269,8 +269,39 @@ class Item(object):
 bld.install_files(install_path, self.get(bld, "target"))
 
 def install_files(self, bld):
+if "rtems_installed" not in bld.env:
+bld.env["rtems_installed"] = {}
+
 for install in self.data["install"]:
-bld.install_files(install["destination"], install["source"])
+# setup destination array if it doesn't exist
+dest = install["destination"]
+if dest not in bld.env.rtems_installed:
+bld.env["rtems_installed"][dest] = []
+
+# build deduplicated install set
+dedup_set = []
+
+for item in install["source"]:
+# search for duplicate installs
+match_found = False
+filename = os.path.basename(item)
+
+for existing in bld.env["rtems_installed"][dest]:
+if existing[0] == filename:
+# duplicate found
+if item != existing[1]:
+bld.fatal(("File installs {} and {} " +
+"target the same location").format(
+item, existing[1]))
+match_found = True
+break
+
+if not match_found:
+dedup_set.append(item)
+bld.env["rtems_installed"][dest].append([filename, item])
+
+if len(dedup_set):
+bld.install_files(dest, dedup_set)
 
 def asm(self, bld, bic, source, target=None):
 if target is None:
-- 
2.30.2

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