[PATCH 4/6] bsps: Move version.c and use bspopts.h

Wed Apr 4 07:55:24 UTC 2018

On 04/04/2018 17:23, Sebastian Huber wrote:
> On 04/04/18 04:23, Chris Johns wrote:
>> Hi Sebastian,
>>
>> I do like the use of shared.am to add sources to a BSP. It hides what is being
>> built and at this point in time I actually feel a clear view in one place is
>> better than another level of indirection.
>>
>> Do you have more sources you wish to add this way?
> 
> Yes, see patch 6. I would like to add the libchip stuff as well and later more
> sources, e.g. bootcard.c.
> 

Makes sense.

>>
>> I sense a number of the shared BSP automake decls could be moving.
>>
>> As stated before I have a dislike of the includes in the build system because
>> they are chaotic and abused. This is not our fault, it is what we inherited.
>> They are hiding too many things and it makes it hard to figure out what is
>> happening. I am currently struggling to understand what is being done by
>> automake and what we do in a number of places and that is not great and this is
>> largely due to all the includes and overloaded use of file names with different
>> functionality, ie local.am.
>>
>> Upon review of the above and my other comments I wonder if the name 'shared.am'
>> and the location in the BSP Makefile.am file makes it look like a build
>> infrastructure file and this is what I am struggling with rather than the
>> purpose of the change. When I saw another file next to local.am I had a little
>> cry.
>>
>> What if the include is positioned before the build infrastructure related
>> includes (with some white space) and files given a specific name, for example:
>>
>>   include $(srcdir)/../../../../../../bsps/shared/source-version.am
> 
> I would like to simplify the BSP build area to use only one Makefile.am
> (c/src/lib/libbsp/@RTEMS_CPU@/@RTEMS_BSP_FAMILY@/Makefile.am and no libraries
> wrapup, etc. I would like to reduce also the copy and paste in these
> Makefile.am. What about:
> 
> include $(srcdir)/../../../../../../bsps/shared/shared-sources.am

This is better. I suppose it depends on how many are going to be added, I know
the shared-bsp-sources and shared-version-source but what about libchip or others?

I am thinking about opening the BSP's makefile and the list says this group of
sources and/or that group. These would be major functional groups.

> 
> In general *-sources.am for files which contain
> 
> libbsp_a_SOURCES += ...
> 
> stuff.

This is a good idea and a step forward.

> 
>>
>> If you plan to add others it does require updating the related BSP Makefile.am
>> files but I doubt this is a big issue other than during these refactoring tasks.
>>
>> Also can we please decide on which automake we are going to use and sort that
>> out before we push any more major changes? It would save needing to retest and
>> maybe fix issues exposed with an update. That would be a wasted effort. I
>> currently doubt the cpukit and BSP tree will have a problem as it stands with
>> some changes and my testing seems to confirm this but the testsuite needs work.
>> If you wish to move the BSP source and maybe change the number of configure's
>> run I feel having a clear direction set is important.
> 
> I am not sure if we should touch the Automake version at the moment. I would
> rather try to simplify the build system with the current Automake version first.

If you can restrict yourself to just automake defined functionality you stand a
chance, if you cannot you step into implementation issues. Please review the
patch below.

I am close to solution for the fstests with the latest automake and this will
mean a number of changes for example removing the imfs_fserror, jffs2_fserror,
etc test directories and moving to building all the file system variants in a
single test directory such as fserror. I do not think this will step on your
toes. Once I have a solution I can craft a patching script.

> I think we need only three Makefile.am and two configure.ac:
> 
> configure.ac
> Makefile.am (builds also cpukit, uses AM_CONDITIONAL for cpukit/score/cpu/*)
> testsuites/Makefile.am (uses AM_CONDITIONAL for each test)

500+ tests in the one file? Really? I hope not.

You cannot have more than one test in a Makefile.am currently. See $(make-exe)
and LINK_OBJS and LINK_LIBS.

Chris

> bsps/@RTEMS_CPU@/@RTEMS_BSP_FAMILY@/configure.ac
> bsps/@RTEMS_CPU@/@RTEMS_BSP_FAMILY@/Makefile.am
> 
>>
>> Repeating my comments here ...
>>
>> I feel we need to update automake to mitigate the dependency risk older versions
>> of automake have or may have with other dependent host programs, for example
>> perl.
>>
>> An example of a breakage with the latest automake is:
>>
>> diff --git a/c/src/configure.ac b/c/src/configure.ac
>> index ff7c7752ed..705fd4af54 100644
>> --- a/c/src/configure.ac
>> +++ b/c/src/configure.ac
>> @@ -125,8 +125,11 @@ RTEMS_BSP_CONFIG_SUBDIR(
>>     [test x"$multilib" = xno])
>>
>>   ## Note: the order of the directories below is essential
>> -AC_CONFIG_SUBDIRS([make])
>> -BSP_SUBDIRS="$BSP_SUBDIRS make"
>> +## Note note: this whole implementation is a bad hack and is broken
>> +## in the latest automake as would be expected, the order of post commands
>> +## has changed so as a result we need to use the "hack" and not the documented
>> +## standard way.
>> +RTEMS_BSP_CONFIG_SUBDIR([make],[make],[],[true])
>>
>>   # Is there code where there should be for this BSP?
>>   RTEMS_CPU_SUBDIRS([lib/libbsp])
>>
>> This is fragile stuff. Just check the comment in the file containing
>> RTEMS_BSP_CONFIG_SUBDIR.
>>
>> Chris
>>
>> On 03/04/2018 23:27, Sebastian Huber wrote:
>>> This patch is a part of the BSP source reorganization.
>>>
>>> Update #3285.
>>> Update #3375.
>>> ---
>>>   c/src/support/version.c => bsps/shared/rtems-version.c |  5 ++++-
>>>   bsps/shared/shared.am                                  |  1 +
>>>   c/src/configure.ac                                     |  2 --
>>>   c/src/support/Makefile.am                              | 10 ----------
>>>   c/src/wrapup/Makefile.am                               |  2 --
>>>   5 files changed, 5 insertions(+), 15 deletions(-)
>>>   rename c/src/support/version.c => bsps/shared/rtems-version.c (83%)
>>>   delete mode 100644 c/src/support/Makefile.am
>>>
>>> diff --git a/c/src/support/version.c b/bsps/shared/rtems-version.c
>>> similarity index 83%
>>> rename from c/src/support/version.c
>>> rename to bsps/shared/rtems-version.c
>>> index 4caf0bd3f7..16f74070bc 100644
>>> --- a/c/src/support/version.c
>>> +++ b/bsps/shared/rtems-version.c
>>> @@ -9,6 +9,8 @@
>>>     #include <rtems/system.h>
>>>   +#include <bspopts.h>
>>> +
>>>   #ifndef RTEMS_VERSION
>>>   #error "Missing RTEMS_VERSION"
>>>   #endif
>>> @@ -23,4 +25,5 @@
>>>   #endif
>>>     const char _RTEMS_version[] =
>>> -  "rtems-" RTEMS_VERSION " (" CPU_NAME "/" CPU_MODEL_NAME "/" RTEMS_BSP ")";
>>> +  "rtems-" RTEMS_VERSION " (" CPU_NAME "/" CPU_MODEL_NAME "/"
>>> +  RTEMS_XSTRING( RTEMS_BSP ) ")";
>>> diff --git a/bsps/shared/shared.am b/bsps/shared/shared.am
>>> index e69de29bb2..854710d804 100644
>>> --- a/bsps/shared/shared.am
>>> +++ b/bsps/shared/shared.am
>>> @@ -0,0 +1 @@
>>> +libbsp_a_SOURCES += ../../../../../../bsps/shared/rtems-version.c
>>> diff --git a/c/src/configure.ac b/c/src/configure.ac
>>> index ff7c7752ed..508f20f18e 100644
>>> --- a/c/src/configure.ac
>>> +++ b/c/src/configure.ac
>>> @@ -134,7 +134,6 @@ AC_SUBST(libbsp_cpu_subdir,$RTEMS_CPU)
>>>     BSP_SUBDIRS="$BSP_SUBDIRS lib/libbsp"
>>>   BSP_SUBDIRS="$BSP_SUBDIRS libchip"
>>> -BSP_SUBDIRS="$BSP_SUBDIRS support"
>>>   BSP_SUBDIRS="$BSP_SUBDIRS wrapup"
>>>     # Build testsuites
>>> @@ -168,7 +167,6 @@ AC_CONFIG_FILES([Makefile],
>>>    MAKE=${MAKE}])
>>>     AC_CONFIG_FILES([
>>> -support/Makefile
>>>   libchip/Makefile
>>>   lib/libbsp/Makefile
>>>   wrapup/Makefile
>>> diff --git a/c/src/support/Makefile.am b/c/src/support/Makefile.am
>>> deleted file mode 100644
>>> index 66c81546c2..0000000000
>>> --- a/c/src/support/Makefile.am
>>> +++ /dev/null
>>> @@ -1,10 +0,0 @@
>>> -include $(top_srcdir)/automake/compile.am
>>> -
>>> -AM_CPPFLAGS += -DRTEMS_BSP=\"@RTEMS_BSP@\"
>>> -
>>> -noinst_LIBRARIES = libsupport.a
>>> -
>>> -libsupport_a_SOURCES = version.c
>>> -libsupport_a_CPPFLAGS = $(AM_CPPFLAGS)
>>> -
>>> -include $(top_srcdir)/automake/local.am
>>> diff --git a/c/src/wrapup/Makefile.am b/c/src/wrapup/Makefile.am
>>> index a52b90df6f..42d283ab6a 100644
>>> --- a/c/src/wrapup/Makefile.am
>>> +++ b/c/src/wrapup/Makefile.am
>>> @@ -13,8 +13,6 @@ TMPINSTALL_FILES = $(PROJECT_LIB)/librtemsbsp.a
>>>     CLEANFILES = o-optimize/librtemsbsp.a
>>>   -SRCS = ../support/libsupport.a
>>> -
>>>   SRCS += ../lib/libbsp/@RTEMS_CPU@/@RTEMS_BSP_FAMILY@/libbsp.a
>>>     SRCS += ../libchip/libflash.a
>>>
> 