Refine the apps build system again

[Xiang Xiao]

Two improvement here:

1.Move the common variable definition into apps/Make.defs

2.Make.defs under sub folder could export CFLAGS/CXXFLAGS to others

Greg, please help review this patchset.

Thanks

Xiang 

[Gregory Nutt]

Reviewed, committed, and pushed.  Later today, after I catch up on all
of the patches, I will start a complete build test to assure that all of
the  current configurations work.

Greg

[Gregory Nutt]

[patacongo]

Hi, Xaio Xiang,

I have been trying to get the buildtest working again and a number of things are now broken under apps/.  I think I will need your help to get them fixed.

First there is this mysterious warning that appears many times during the build test:

<command-line>: warning: "main" redefined
<command-line>: note: this is the location of the previous definition

Then there are two configurations that fail building old applications under apps/examples:

Configuration: clicker2-stm32:xbee-6lowpan
Error.  The file config.h is not created.  This should be created during by the context target.  The context target of every makefile should have been called early in initialization.  So every file that includes config.h fails like:

CC:  nettest_cmdline.c
arm-none-eabi-gcc -c -fno-builtin -Wall -Wstrict-prototypes -Wshadow -Wundef -Os -fno-strict-aliasing -fno-strength-reduce -fomit-frame-pointer -mcpu=cortex-m4 -mthumb -mfpu=fpv4-sp-d16 -mfloat-abi=hard -I. -isystem "D:\Spuda\Documents\projects\nuttx\master\nuttx\include"    -pipe -I "D:\Spuda\Documents\projects\nuttx\master\apps\include"   nettest_cmdline.c -o  nettest_cmdline.o
nettest_cmdline.c:40:10: fatal error: config.h: No such file or directory
 #include "config.h"
          ^~~~~~~~~~
compilation terminated.

Configuration: b-l475e-iot01a:spirit-6lowpan
Error.  The file config.h is again not created but this time for apps/examples/udp.  Again every file that includes config.h fails.  This appears to be the same kind of problem, but for a different example.

There are still several hundred configurations that have not yet been compiled, so there may still be more problems.

Thanks in advance for your help,

Greg

[Gregory Nutt]

Configuration: clicker2-stm32:xbee-6lowpan
Error.  The file config.h is not created.  This should be created during by the context target.  The context target of every makefile should have been called early in initialization.

So the explanation seems to be that the recent changes to the apps/ directory has broken the build of examples that require that the context target be invoked before the binary is built???  How could that happen?

Greg

[patacongo]

Then there are two configurations that fail building old applications under apps/examples:

...

 

There are still several hundred configurations that have not yet been compiled, so there may still be more problems.

Yes, the problem is much more extensive.  ALL board configurations that use apps/examples/nettest and apps/examples/udp are broken.  There are many.

[patacongo]

This appears to be the commit that broke these builds:  e806097c70084ba98ff2ed1d018afbc126e533ae

With that commit, config.h is no longer created and all of the compiles fail.  I can see that clearly in the apps/nettest/Makefile case where the config.h target gets conditioned out completely.  That is wrong but I don't understand how to fix it in the context of the new build logic.  This must be fixed.

I believe this failure occurs only for the cases where the host binary is not compiler but that is a lot of configurations:  Here are the known failing builds:

clicker2-stm32:xbee-6lowpan

b-l475e-iot01a:spirit-6lowpan

b-l475e-iot01a:spirit-starpoint

clicker2-stm32:mrf24j60-6lowpan

clicker2-stm32:mrf24j60-starpoint

And probably more.  I have not yet made it all the way through the build tests.

[patacongo]

I believe this failure occurs only for the cases where the host binary is not compiled but that is a lot of configurations:  Here are the known failing builds:

clicker2-stm32:xbee-6lowpan

b-l475e-iot01a:spirit-6lowpan

b-l475e-iot01a:spirit-starpoint

clicker2-stm32:mrf24j60-6lowpan

clicker2-stm32:mrf24j60-starpoint

And probably more.  I have not yet made it all the way through the build tests.

These tests do not use the host binary.  The all do UDP and nettest using the server on one radio and the client on another radio.

[patacongo]

I believe that the clean target is also broken for these apps/examples.  That is because the original conditional logic was divided up into several small chunks to allow parts that were needed when there is no host app and parts that are not needed.  Replacing that with with the single large conditional that spans all of the logic loses the creation of the necessary config.h header file but also loses cleaning of the config.h header file.  Both are errors.

My initial attempts to back out the problem commit did not work however.  So your help would be appreciated.

[patacongo]

My initial attempts to back out the problem commit did not work however.  So your help would be appreciated.

Okay. I finally got commit backed out properly and now apps/examples/udp and apps/examples/nettest will again build correctly when there is no host binary:

commit 2830892424da13e78b4fc4da9030d5734ac25699
Author: Gregory Nutt <gn...@nuttx.org>
Date:   Fri Oct 18 16:24:43 2019 -0600

    apps/examples/udp and nettest:  Back out parts of commit e806097c70084ba98ff2ed1d018afbc126e533ae that broke the build of these examples.

I will start the build test again in the morning and see how far I can get.  A lot has been broken.

[patacongo]

Also:

commit 8cb4896bdb07e778da87732600b30bc0a452a0a8
Author: Gregory Nutt <gn...@nuttx.org>
Date:   Fri Oct 18 16:47:57 2019 -0600

    apps/examples/flowc, tcpblaster, and udpblaster:  Back out more changes that broke the makefiles fixed by commit 2830892424da13e78b4fc4da9030d5734ac25699.  This applies the same fix to other Makefiles that follow that same pattern and seem to have the same breakage.

[Gregory Nutt]

I still do see many of these warnings in the build.  It would be nice to have these fixed too:

<command-line>: warning: "main" redefined
<command-line>: note: this is the location of the previous definition

--
You received this message because you are subscribed to the Google Groups "NuttX" group.
To unsubscribe from this group and stop receiving emails from it, send an email to nuttx+un...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/nuttx/a1a08132-39ae-4194-b7a5-2770f0eaa90b%40googlegroups.com.

[Xiang Xiao]

Very thanks for your help to fix my fault for config.h, could you send the more context information about "main redefined" issue?

The attached patch fix one link issue when I reproduce these compile error.

Thanks

Xiang

On Saturday, October 19, 2019 at 7:17:20 AM UTC+8, patacongo wrote:

I still do see many of these warnings in the build.  It would be nice to have these fixed too:

<command-line>: warning: "main" redefined
<command-line>: note: this is the location of the previous definition

On 10/18/2019 4:48 PM, patacongo wrote:

Also:

commit 8cb4896bdb07e778da87732600b30bc0a452a0a8
Author: Gregory Nutt <gn...@nuttx.org>
Date:   Fri Oct 18 16:47:57 2019 -0600

    apps/examples/flowc, tcpblaster, and udpblaster:  Back out more changes that broke the makefiles fixed by commit 2830892424da13e78b4fc4da9030d5734ac25699.  This applies the same fix to other Makefiles that follow that same pattern and seem to have the same breakage.

--
You received this message because you are subscribed to the Google Groups "NuttX" group.

To unsubscribe from this group and stop receiving emails from it, send an email to nu...@googlegroups.com.

[Gregory Nutt]

> Very thanks for your help to fix my fault for config.h, could you send
> the more context information about "main redefined" issue?

I will try to get more information.  I am not sure why or where it is
happening.  It looks like it is coming from bash code in a Makefile.

> The attached patch fix one link issue when I reproduce these compile
> error.

Committed, Thanks!

Greg

[Gregory Nutt]

Hi, Xiang,

Very thanks for your help to fix my fault for config.h, could you send the more context information about "main redefined" issue?

I will try to get more information.  I am not sure why or where it is happening.  It looks like it is coming from bash code in a Makefile.

No, it is apparently coming from GCC.  I was able to replicate the problem in this way:

tools/configure.sh -c open1788:knxterm
make staging/libapps.a V=1 1>log 2>&1

Then in the resulting log file, I can clearly see the problem:

CC:  nettest_target2.c
arm-none-eabi-gcc -c -fno-builtin -Wall -Wstrict-prototypes -Wshadow -Wundef -Os -fno-strict-aliasing -fno-strength-reduce -fomit-frame-pointer -mcpu=cortex-m4 -mthumb -mfpu=fpv4-sp-d16 -mfloat-abi=hard -I. -isystem "D:\Spuda\Documents\projects\nuttx\master\nuttx\include"    -pipe -I "D:\Spuda\Documents\projects\nuttx\master\apps\include" -Dmain=nettest1_main -Dmain=nettest2_main   nettest_target2.c -o  nettest_target2.o

<command-line>: warning: "main" redefined
<command-line>: note: this is the location of the previous definition

The problem is that it is trying to assign two main() names.  The error does not repeat if you rebuild libapps.a because no files are compiled, but can be regenerated this way:

make apps_distclean (or maybe apps_clean)
make staging/libapps.a V=1 1>log 2>&1

I am not sure what the fix is.  Perhaps we will need two makefiles for these cases?  You understand the intent of the design better than I do.

Today I will run the build test and try to provide a more complete list of all of the configurations that generate the problem.

Greg

[Xiang Xiao]

Greg, please try the attached patch which should fix this issue.

Thanks

Xiang

[Gregory Nutt]

On 10/19/2019 11:48 AM, Xiang Xiao wrote:
> Greg, please try the attached patch which should fix this issue.

This will eliminate the warning.  But I don't think this will eliminate
the problem.

In these tests, there are usually two parts:  One part that runs on
target and one part that runs on the host.  In that case, there is only
one target main on the command line.  For the cast of the nettest
example, the one that runs on the host will have main() and the one that
runs on the target will have nettest_main().

But there are also special configurations where both sides of the test
run on the host.  In that case, there is no host main(), but there needs
to be two target main functions.  For the case of the nettest example,
these need to be netest1_main() and nettest2_main().  Using the
"firstword" command will pick off the first one, nettest1, and discard
the second one, nettest2.  Thus both entry points would be incorrectly
named nettest1_main().

Given the changes to the apps/ make, I think that something more complex
in the example Makefiles is needed.  I am thinking that the single
Makefile needs to be separated int Makefile, Makefile.host,
Makefile.nettest1, Makefile.netest2.  These individual makes can can
then be invoked.  Each can include Application.mk and each can have a
unique target entry point name.

Greg

[Xiang Xiao]

On Sunday, October 20, 2019 at 1:58:13 AM UTC+8, patacongo wrote:

On 10/19/2019 11:48 AM, Xiang Xiao wrote:
> Greg, please try the attached patch which should fix this issue.

This will eliminate the warning.  But I don't think this will eliminate
the problem.

In these tests, there are usually two parts:  One part that runs on
target and one part that runs on the host.  In that case, there is only
one target main on the command line.  For the cast of the nettest
example, the one that runs on the host will have main() and the one that
runs on the target will have nettest_main().

But there are also special configurations where both sides of the test
run on the host.  In that case, there is no host main(), but there needs
to be two target main functions.  For the case of the nettest example,
these need to be netest1_main() and nettest2_main().  Using the
"firstword" command will pick off the first one, nettest1, and discard
the second one, nettest2.  Thus both entry points would be incorrectly
named nettest1_main().

But filter-out call in the 3rd line will remove the first word, so the new function name will be used in the new iteration:

$(eval $<_CXXFLAGS += ${shell $(DEFINE) "$(CXX)" main=$(firstword $(MAINNAME))})

$(eval $<_CXXELFFLAGS += ${shell $(DEFINE) "$(CXX)" main=$(firstword $(MAINNAME))})

$(eval MAINNAME=$(filter-out $(firstword $(MAINNAME)),$(MAINNAME)))

 

Given the changes to the apps/ make, I think that something more complex
in the example Makefiles is needed.  I am thinking that the single
Makefile needs to be separated int Makefile, Makefile.host,
Makefile.nettest1, Makefile.netest2.  These individual makes can can
then be invoked.  Each can include Application.mk and each can have a
unique target entry point name.

Yes, the separated Makefile could resolve this problem too, actually I discussed this appraoch with An Chao in the last year.

But finally, he decided to enhance PROGNAME to support the mulitple element because:

1.Keep the whole Makefile structure

2.Avoid Makefile duplication

 

Greg

[Gregory Nutt]

But filter-out call in the 3rd line will remove the first word, so the new function name will be used in the new iteration:

$(eval $<_CXXFLAGS += ${shell $(DEFINE) "$(CXX)" main=$(firstword $(MAINNAME))})

$(eval $<_CXXELFFLAGS += ${shell $(DEFINE) "$(CXX)" main=$(firstword $(MAINNAME))})

$(eval MAINNAME=$(filter-out $(firstword $(MAINNAME)),$(MAINNAME)))

 

Okay, I will give it a try!

Thanks for the explanation.

Greg

[patacongo]

With your last change plus a few more that I did, the ARM build test completes with no errors and no new warnings.  There are some new board configurations that have some inappropriate warnings.  But all of the previously existing ARM configurations build with no differences in the output.

Thanks for your help!

Greg