[PATCH 2/5] build: Use CSafeLoader if available

[Gedare Bloom]

On Tue, May 23, 2023 at 12:04 AM Sebastian Huber
<sebastian.huber at embedded-brains.de> wrote:
>
>
>
> On 23.05.23 02:42, Chris Johns wrote:
> > On 22/5/2023 6:29 pm, Sebastian Huber wrote:
> >> On 22.05.23 01:48, Chris Johns wrote:
> >>>> The support for the CSafeLoader is just about 60 lines of additional code.
> >>>> It would be a nice improvement for systems supporting this feature. I don't have
> >>>> time to work on adding PyYAML with libyaml to the RTEMS Tools right now and I
> >>>> think this would open another maintenance issues. If someone would be unable to
> >>>> get a yaml module with a CSafeLoader, then he would be no longer able to build
> >>>> RTEMS.
> >>> It is not about the size of the implementation, it is about making 2 classes of
> >>> performance, those with and those without csafe. I am a firm believer all
> >>> developers and especially core developers need to be using the exact same tools
> >>> and code our community users are using. We did not always do this and it
> >>> resulted in the developers being disconnected from the user experience and I
> >>> decided I would do what I could to avoid this happening again.
> >>
> >> I really don't see the issue here. It is just the way to load the items from the
> >> file system. This is a very isolated task in the build and having two ways to do
> >> this is not a big deal from my point of view.
> >
> > Great, lets not go down this path and we all use the same process.
>
> The CSafeLoader approach makes my work more efficient, so I will
> definitely use it.
>
> >
> >>> In regards to PyYAML there are a few basic issues with it that I am not sure
> >>> about. The package can be built without support for the C library even if it is
> >>> installed. I cannot tell if a pip installed version is using the C YAML package
> >>> or not. I would like more certainty across our supported hosts before agreeing
> >>> to us using it.
> >>
> >> I think it is just too complicated to make sure that the user has a PyYAML with
> >> the CSafeLoader available when RTEMS is built. A more robust approach is a fall
> >> back to the current implementation if needed.
> >

Without a reliable way to ensure all users can equivalently use the
tools, I see the topic as not worth fighting a lot about. I have seen
projects that brought in build/configure time optional packages for
performance tuning and optimization with fallbacks, e.g., tcmalloc
instead of malloc. In those cases, it becomes harder to help debug
problems since it introduces another branching point to determine what
might go wrong. Although I'm sure this can be resolved given time, it
doesn't sound like anyone has time currently to integrate the
CSafeLoader in a way that makes certain that users get/use what we
expect.

I would suggest we can revisit the topic later, if Sebastian is
satisfied he can carry his own local optimization around for now. I
think we have probably all done things like that in the past. I
appreciate the discussion and that the support to use it was brought,
but unless the build times become a general problem for more
developers and the user base, I don't find this debate fruitful.

Gedare

> > Maybe we move away from YAML for the build system data? Amar and I originally
> > had python and INI files.
>
> This discussion started with a proposal to switch to JSON:
>
> https://lists.rtems.org/pipermail/devel/2023-April/075082.html
>
> The INI format is a simple key-value format with no support for
> hierarchical data, so not suitable from my point of view.
>
> Using Python code could be an option, however, this gives the writer of
> the files more freedom and room for creativity. On lesson learnt from
> converting the autoconf/automake build to the new build system is, that
> this may cause some issues.
>
> --
> embedded brains GmbH
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.huber at 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/