"smart build" and comments

Discussions and support about ChibiOS/RT, the free embedded RTOS.
Thargon
Posts: 135
Joined: Wed Feb 04, 2015 5:03 pm
Location: CITEC, Bielefeld University, germany
Has thanked: 15 times
Been thanked: 24 times
Contact:

"smart build" and comments

Postby Thargon » Thu May 24, 2018 2:35 pm

Hi,

I just encountered a very bad and as it seems not well documented limitation of the "smart build" feature. If the variable USE_SMART_BUILD is set to "yes" in the Makefile, os/rt/rt.mk will parse the chconf.h file. This parsing is incorrect, though, as it searches for all lines containing the phrase '#define', not starting with this phrase. As a result, lines like

Code: Select all

//#define CH_CFG_USE_EVENTS
will still be detected as valid by rt.mk.
A solution to the given example would be to modify the according line in rt.mk from

Code: Select all

CHCONF := $(strip $(shell cat chconf.h | egrep -e "\#define"))
to

Code: Select all

CHCONF := $(strip $(shell cat chconf.h | egrep -e "^\#define"))
But then again, actually valid lines like

Code: Select all

/*commend*/#define CH_CFG_USE_EVENTS
would not be detected anymore.

My situation is actually a bit more complex and reveals another strong limitation of the smart build feature. I have multiple projects with mostly redundant chconf.h files. Hence I wanted to introduce a common header, which is included by each chconf.h. Since smart build will only scan the 'main' chconf.h of each project but not any included files, it completely fails in this case.
I defined

Code: Select all

#define CH_CFG_USE_EVENTS                   TRUE
in my common configuration header and commented the according line in chconf.h:

Code: Select all

//#define CH_CFG_USE_EVENTS                   TRUE
This compiles completely fine, but fails as soon as I delete the commented line from chconf.h. As you can imagine it took me some time to understand why deleting a comment causes linking errors. The reason is, that the C preprocessor will detect the define correctly and all function declarations are 'activated'. rt.mk, however, will prevent the according .c file(s) to be compiled, so the linker eventually encounters undefined references. Thanks to rt.mk erroneous file parsing, keeping the comment hides this issue since rt.mk will still 'activate' the required .c file(s).

Given these strong limitations and the fact that smart build merely skips compilation of no more than up to 13 "empty" .c files (content is ignored due to the defines), I wonder about the benefit of this feature. If you build a single project, there will probably hardly be any speedup measurable, but if you build multiple projects the feature is very prone to break compilation (or rather linking). Personally, I probably spent more time in searching for the cause of my issues than the feature ever saved me. So in my opinion, smart build should be removed from ChibiOS.

Thomas

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: "smart build" and comments

Postby Giovanni » Thu May 24, 2018 4:27 pm

Hi,

I understand your point, however:

1) Commenting out the defines with // is illegal, latest code would complain about that and not even compile because now chconf.h is checked for integrity. If a definition is missing then you get an #error.

2) It is an option in the build system, the fact that does not fit your needs does not mean it should be removed for everybody. It is something that saves a lot of time when compiling on slow hardware.

Giovanni

Thargon
Posts: 135
Joined: Wed Feb 04, 2015 5:03 pm
Location: CITEC, Bielefeld University, germany
Has thanked: 15 times
Been thanked: 24 times
Contact:

Re: "smart build" and comments

Postby Thargon » Thu May 24, 2018 4:53 pm

Giovanni wrote:1) Commenting out the defines with // is illegal, latest code would complain about that and not even compile because now chconf.h is checked for integrity. If a definition is missing then you get an #error.
If I include another header where all those defines exist, that should not be illegal. I think it is a bigger issue to use preprocessor directive with other tools than the preprocessor, which do not handle those directives properly. This is a very bad way of meta programming, imho.

Giovanni wrote:2) It is an option in the build system, the fact that does not fit your needs does not mean it should be removed for everybody. It is something that saves a lot of time when compiling on slow hardware.
That is what I doubt, although I have no numbers here (do you?). In the very best case, smart build omits 13 files to be accessed. Considering the number of files that are involved for even very simple projects (makefiles, headers and sources), and how often some of these are included, I doubt that these 13 will make any difference. Furthermore, with smart build turned off, these files are just accessed, but their content is ignored thanks to the definition checks, and thus they have minimum impact on CPU cost for compilation.

I understand that smart build might help some people with exceptional slow hardware (it might, but I doubt that it does), but in my opinion it probably introduces even more trouble to other people, who use ChibiOS in more complex projects. Since I am part of the latter group my view is biased, but maybe you could still reconsider the benefits and drawbacks of smart build ;) In general it is not a bad feature to skip unneeded files entirely, but the way it is done by smart build could be improved.

- Thomas

apmorton
Posts: 36
Joined: Fri Sep 29, 2017 10:26 am
Been thanked: 16 times

Re: "smart build" and comments

Postby apmorton » Sat May 26, 2018 6:20 am

Perhaps a better way to handle smart-build would be something like this in a makefile target:

Code: Select all

echo "#include <ch.h>" | $(CC) -dM -E -x c-header $(CFLAGS) $(TOPT) -I. $(IINCDIR) - -o "smartbuild.h"


this will output a file called "smartbuild.h" which contains all preprocessor definitions as seen by the compiler when including ch.h.

You could then reliably grep for defines to use with smartbuild and have it be accurate no matter where the definitions come from.


Return to “ChibiOS/RT”

Who is online

Users browsing this forum: No registered users and 11 guests