[riot-notifications] [RIOT-OS/RIOT] Pr/make/exports/remove flash debug reset (#11554)

Gaƫtan Harter notifications at github.com
Mon May 20 20:21:11 CEST 2019


### Contribution description

This removes `export` for all variables related to `flash/reset/debug`.
They do not need to be exported as they are all evaluated in `Makefile.include` or a file included by it.

This does not remove export `PORT` for the moment as it is easier to review it on its own as it will have a different review scheme

### Testing procedure

There is not real testing procedure except, it would all still be working, but not really realistic for testing.


Better do an in depth review to find issues.
If any issue happened because of this, it could be easier to fix the missing case.


Running `make flash/make reset/make debug` still works on one of your boards.
At least it should not break `CI`.

### Review prodecure 

The only usages of the variables are in `Makefile.include` or `.inc.mk` files:

```
git grep -e '$(FFLAGS)' -e '$(FLASHER)' -e '$(RESET)' -e '$(RESET_FLAGS)' -e '$(DEBUGGER)' -e '$(DEBUGGER_FLAGS)' -e '$(DEBUGSERVER)' -e '$(DEBUGSERVER_FLAGS)' -e '$(MSPDEBUGFLAGS)'
Makefile.include:  $(call check_cmd,$(FLASHER),Flash program)
Makefile.include:  $(FLASHER) $(FFLAGS)
Makefile.include:       $(call check_cmd,$(DEBUGGER),Debug program)
Makefile.include:       $(DEBUGGER) $(DEBUGGER_FLAGS)
Makefile.include:       $(call check_cmd,$(DEBUGSERVER),Debug server program)
Makefile.include:       $(DEBUGSERVER) $(DEBUGSERVER_FLAGS)
Makefile.include:       $(call check_cmd,$(RESET),Reset program)
Makefile.include:       $(RESET) $(RESET_FLAGS)
boards/common/msb-430/Makefile.include:FFLAGS = $(MSPDEBUGFLAGS) "prog $(HEXFILE)"
boards/common/msb-430/Makefile.include:DEBUGSERVER = $(FLASHER)
boards/common/msb-430/Makefile.include:DEBUGSERVER_FLAGS = $(MSPDEBUGFLAGS) gdb
boards/common/msba2/Makefile.include:FLASHDEPS += $(if $(findstring $(LPC2K_PGM),$(FLASHER)),$(LPC2K_PGM))
boards/native/Makefile.include:ifeq ($(shell basename $(DEBUGGER)),lldb)
boards/native/Makefile.include: $(eval DEBUGGER_FLAGS := -ex "target remote | vgdb --pid=$(VALGRIND_PID)" $(DEBUGGER_FLAGS))
boards/native/Makefile.include: $(DEBUGGER) $(DEBUGGER_FLAGS)
boards/teensy31/Makefile.include:ifeq ($(TEENSY_LOADER),$(FLASHER))
makefiles/info.inc.mk:  @echo 'FLASHER: $(FLASHER)'
makefiles/info.inc.mk:  @echo 'FFLAGS:  $(FFLAGS)'
makefiles/info.inc.mk:  @echo 'DEBUGGER:       $(DEBUGGER)'
makefiles/info.inc.mk:  @echo 'DEBUGGER_FLAGS: $(DEBUGGER_FLAGS)'
makefiles/info.inc.mk:  @echo 'DEBUGSERVER:       $(DEBUGSERVER)'
makefiles/info.inc.mk:  @echo 'DEBUGSERVER_FLAGS: $(DEBUGSERVER_FLAGS)'
makefiles/info.inc.mk:  @echo 'RESET:       $(RESET)'
makefiles/info.inc.mk:  @echo 'RESET_FLAGS: $(RESET_FLAGS)'
makefiles/murdock.inc.mk:FLASHFILE ?= $(filter $(HEXFILE) $(ELFFILE:.elf=.bin) $(ELFFILE),$(FFLAGS))
makefiles/tools/avrdude.inc.mk:DEBUGGER = $(DIST_PATH)/debug.sh $(DEBUGSERVER_FLAGS) $(DIST_PATH) $(DEBUGSERVER_PORT)
makefiles/tools/bossa.inc.mk:ifeq ($(RIOTTOOLS)/bossa/bossac,$(FLASHER))
makefiles/tools/edbg.inc.mk:ifeq ($(RIOT_EDBG),$(FLASHER))
```

No match when using `${}`
```
git grep -e '${FFLAGS}' -e '${FLASHER}' -e '${RESET}' -e '${RESET_FLAGS}' -e '${DEBUGGER}' -e '${DEBUGGER_FLAGS}' -e '${DEBUGSERVER}' -e '${DEBUGSERVER_FLAGS}' -e '${MSPDEBUGFLAGS}'
```

#### Only required changes are changed

The only changes are `[-export-]` and comments `{+#+}`

```git diff --color=auto --word-diff c439346f6 '**' ':!dist/tools/buildsystem_sanity_check/check.sh'```


Only the right amount of lines have been changed

```
git diff  --stat  c439346f6 '**' ':!dist/tools/buildsystem_sanity_check/check.sh'
...
 28 files changed, 181 insertions(+), 181 deletions(-)
```

This includes:

* 170 `export` changed.
* 8 modifications in `makefiles/vars.inc.mk`
* 2 lines for `MSPDEBUGFLAGS` (that I did not added to build_system_check as really specific)
* 1 newline that is reported in `cpu/esp32/Makefile.include` for `PREFFFLAGS`


And from the `buildsystem_sanity_check` output, on the original commit c439346f6

```
git checkout acb839ffa0c93c727f619d888a0c27d189fcdd5d -- dist/tools/buildsystem_sanity_check/check.sh
./dist/tools/buildsystem_sanity_check/check.sh  | grep -v 'Invalid build system patterns found by' | wc -l
170
```

Matches the number of `export` changed


### Issues/PRs references

Part of https://github.com/RIOT-OS/RIOT/issues/10850
In depth cleanup required for https://github.com/RIOT-OS/RIOT/pull/10440
You can view, comment on, or merge this pull request online at:

  https://github.com/RIOT-OS/RIOT/pull/11554

-- Commit Summary --

  * dist/tools/buildsystem_sanity_check: handle vars exported in vars.inc.mk
  * boards/tools: remove exporting FLASHER/FFLAGS
  * dist/tools/buildsystem_sanity_check: check no FLASHER/FFLAGS exports
  * boards/tools: remove exporting RESET/RESET_FLAGS
  * dist/tools/buildsystem_sanity_check: check no RESET/RESET_FLAGS exports
  * boards/tools: remove exporting DEBUG*
  * dist/tools/buildsystem_sanity_check: check no DEBUG* exports
  * boards/tools: remove exporting MSPDEBUGFLAGS
  * boards/tools: remove exporting PREFLASH/FLASHDEPS
  * dist/tools/buildsystem_sanity_check: check no PREFLASH* exports

-- File Changes --

    M boards/calliope-mini/Makefile.include (10)
    M boards/cc2538dk/Makefile.include (18)
    M boards/chronos/Makefile.include (4)
    M boards/common/esp32/Makefile.include (2)
    M boards/common/esp8266/Makefile.include (2)
    M boards/common/msb-430/Makefile.include (16)
    M boards/common/remote/Makefile.include (20)
    M boards/common/stm32f103c8/Makefile.include (10)
    M boards/common/wsn430/Makefile.include (4)
    M boards/f4vi1/Makefile.include (10)
    M boards/hifive1/Makefile.include (2)
    M boards/mbed_lpc1768/Makefile.include (10)
    M boards/microbit/Makefile.include (10)
    M boards/native/Makefile.include (10)
    M boards/nz32-sc151/Makefile.include (8)
    M boards/opencm904/Makefile.include (10)
    M boards/openmote-b/Makefile.include (18)
    M boards/openmote-cc2538/Makefile.include (4)
    M boards/spark-core/Makefile.include (8)
    M boards/telosb/Makefile.include (4)
    M boards/z1/Makefile.include (4)
    M cpu/esp32/Makefile.include (62)
    M cpu/esp8266/Makefile.include (24)
    M dist/tools/buildsystem_sanity_check/check.sh (24)
    M makefiles/tools/bossa.inc.mk (4)
    M makefiles/tools/jlink.inc.mk (16)
    M makefiles/tools/openocd.inc.mk (16)
    M makefiles/tools/pyocd.inc.mk (16)
    M makefiles/tools/uniflash.inc.mk (24)
    M makefiles/vars.inc.mk (16)

-- Patch Links --

https://github.com/RIOT-OS/RIOT/pull/11554.patch
https://github.com/RIOT-OS/RIOT/pull/11554.diff

-- 
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/pull/11554
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190520/7bf1249a/attachment-0001.html>


More information about the notifications mailing list