[riot-notifications] [RIOT-OS/RIOT] Tracking: remove harmfull use of `export` in make and immediate evaluation (#10850)

Gaëtan Harter notifications at github.com
Wed Jan 23 17:55:53 CET 2019


#### Description

Many variables are exported using `export VARIABLE` without even knowing why, the impact, or even be needed. In the same time, many variables are evaluated using `:=` instead of using `deferred` variables.

Guess why calling `make clean` is slow ? All these variables/shell call must be evaluated for each target called even if the targett is not using it at all.

I tried preventing changing these without reasons, but keeping them in this wrong state by default, is causing me issues in several places. I want to start getting rid of them on a methodical way instead of currently working it around.

#### Steps

* List variables where export or direct evaluation can directly be removed here == only used inside `Makefile.include and remove them
  + [ ] LINK/LINKFLAGS
  + [ ] PORT/FFLAGS/TERMPROG/TERMFLAGS/FLASHER
* Only export in `makefiles/vars.inc.mk` for variables that currently really need to be exported  
* Variables that need to be exported to sub-builds need a mechanism to say who gets it
* Variables that are transparently given to scripts even if only one needs it
  + [ ] GIT_CACHE_DIR
  + flashing tools specific variables which are exported in `board` instead of in the `flasher`



This has different consequences:

##### Evaluation when not needed

* Using an `export` on a variable means the value will be evaluated even for rules that do not use the variable.
This causes issues for example with listing `tty` devices, evaluating `INCLUDES` and getting `GCC_INCLUDES_DIRS`

* It also causes evaluation on the host machine when only the docker container should be doing it. It happened with `avr-ld` for example, some `checks` being performed on the host machine, saying `objcopy not found`.


##### Sub builds

When trying to call `make` inside of `make` many variables from the first build are sent to the second one.


##### Variables that can change a build

As everything is exported, it is hard to know what can have an impact on a build result.
Having it in control would simplify documenting what should and what should not be available.

##### The default behavior has become harmful

The default case has been to always use `:=` or `export` even when it does not make sense, and it gets merged without even reviewing why there is an `export` there.

```
export PRIVATE_VAR_FOR_SCRIPT = foo_bar
OTHER_VAR_ONLY_USE_OF_PRIVATE_VAR_FOR_SCRIPT = $(PRIVATE_VAR_FOR_SCRIPT)
```

#### Examples

* `GLOBAL_GOALS` has it should not evaluate the rest of the build system because it is `slow` or causes isssues
* https://github.com/RIOT-OS/RIOT/pull/10840
* https://github.com/RIOT-OS/RIOT/pull/7695#issuecomment-361863987




-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/issues/10850
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190123/e91353d4/attachment.html>


More information about the notifications mailing list