Skip to content

unexport TERM* variables#11064

Merged
jcarrano merged 3 commits intoRIOT-OS:masterfrom
cladmi:pr/bug/export/term_variables
Feb 25, 2019
Merged

unexport TERM* variables#11064
jcarrano merged 3 commits intoRIOT-OS:masterfrom
cladmi:pr/bug/export/term_variables

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Feb 25, 2019

Contribution description

TERMPROG and TERMFLAGS variables do not need to be exported as they are
used directly by a make receipe.

Exporting them prevents overwriting 'RIOT_TERMINAL' in the test.

Other uses to unexport:

There is one remaining usage of export TERMFLAGS but it should be handled alone as it does not make sense as it does not handle RIOT_TERMINAL. I will add it to the tracking issue.

git grep -e 'export TERM'
boards/nz32-sc151/Makefile.include:export TERMFLAGS = -p $(PORT)

Testing procedure

Review

We can see that there are not usages in other places than recipes that are declared in Makefile.include:

git grep -e '$(TERMPROG)' -e '$(TERMFLAGS)' -e '${TERMPROG}' -e '${TERMFLAGS}'
Makefile.include:       $(call check_cmd,$(TERMPROG),Terminal program)
Makefile.include:       $(TERMPROG) $(TERMFLAGS)
boards/native/Makefile.include:TERMFLAGS := $(PORT) $(TERMFLAGS)
boards/native/Makefile.include:  export DEBUGGER_FLAGS = -- $(ELFFILE) $(TERMFLAGS)
boards/native/Makefile.include:  export DEBUGGER_FLAGS = -q --args $(ELFFILE) $(TERMFLAGS)
makefiles/info.inc.mk:  @echo 'TERMPROG:  $(TERMPROG)'
makefiles/info.inc.mk:  @echo 'TERMFLAGS: $(TERMFLAGS)'

There are no more exports of TERMFLAGS or TERMPROG except the one already mentioned earlier:

git grep -e 'export TERM'
boards/nz32-sc151/Makefile.include:export TERMFLAGS = -p $(PORT)

Testing make term

It is still working for both native and boards. You can try

make -C examples/hello-world/ all term
BOARD=samr21-xpro make -C examples/default all term

The term-gprof for native is not working both in master and with this PR.

Overwriting RIOT_TERMINAL in testrunner:

The usage within make test can be checked with this diff.
With it, running a test on a board should use socat

diff --git a/dist/pythonlibs/testrunner/__init__.py b/dist/pythonlibs/testrunner/__init__.py
index 67489f00c..06f086a02 100755
--- a/dist/pythonlibs/testrunner/__init__.py
+++ b/dist/pythonlibs/testrunner/__init__.py
@@ -38,6 +38,7 @@ def find_exc_origin(exc_info):
 
 def run(testfunc, timeout=10, echo=True, traceback=False):
     env = os.environ.copy()
+    env['RIOT_TERMINAL'] = 'socat'
     child = pexpect.spawnu("make term", env=env, timeout=timeout, codec_errors='replace')
BOARD=samr21-xpro  make -C tests/bloom_bytes/ flash test
make: Entering directory '/home/harter/work/git/RIOT/tests/bloom_bytes'
...
Programming..................................................... done.
Verification..................................................... done.
socat - open:/dev/ttyACM0,b115200,echo=0,raw
adding 512 elements took 262ms
checking 10000 elements took 2468ms

267 elements probably in the filter.
9733 elements not in the filter.
 false positive rate.

All done!
main(): Tmain(): This is RIOT! (Version: 2019.04-devel-269-g3b11-pr/bug/export/term_variables)
Testing Bloom filter.

m: 4096 k: 8

adding 512 elements took 262ms
checking 10000 elements took 2468ms

267 elements probably in the filter.
9733 elements not in the filter.
 false positive rate.

All done!

make: Leaving directory '/home/harter/work/git/RIOT/tests/bloom_bytes'

Issues/PRs references

Tracking issue #10850
Was an effective issue while working on #10952

@cladmi
makefiles/vars.inc.mk: unexport TERM* variables
d0cb6d2 
TERMPROG and TERMFLAGS variables do not need to be exported as they are
used directly by a make receipe.

Exporting them prevents overwriting 'RIOT_TERMINAL' in the test.
@cladmi
makefiles/tools/serial.inc.mk: unexport TERM* variables
20ce4db 
TERMPROG and TERMFLAGS variables do not need to be exported as they are
used directly by a make receipe.

Exporting them prevents overwriting 'RIOT_TERMINAL' in the test.
@cladmi
boards/native: unexport TERM* variables
3b119cc 
TERMPROG and TERMFLAGS variables do not need to be exported as they are
used directly by a make receipe.
@cladmi cladmi added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 25, 2019
@cladmi cladmi requested review from aabadie and jcarrano February 25, 2019 16:42
@cladmi cladmi added the CI: run tests If set, CI server will run tests on hardware for the labeled PR label Feb 25, 2019
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Feb 25, 2019

Please tell me if you see other testing procedure I missed.

@cladmi cladmi mentioned this pull request Feb 25, 2019
Closed
19 tasks
@jcarrano
Copy link
Copy Markdown
Contributor

jcarrano commented Feb 25, 2019

@aabadie, you added the boards/nz32-sc151, but it looks kind of broken in the way it sets TERMFLAGS (it won't work if one changes RIOT_TERMINAL).

@cladmi cladmi mentioned this pull request Feb 25, 2019
Merged
2 tasks
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Feb 25, 2019

I am doing a pull request for the nz32 alone.

jcarrano
jcarrano approved these changes Feb 25, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jcarrano jcarrano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested make term in samr21. It still works. Also, the exports were clearly unnecessary.

@jcarrano jcarrano self-requested a review February 25, 2019 17:01
@jcarrano jcarrano merged commit 7e037d7 into RIOT-OS:master Feb 25, 2019
@cladmi cladmi added this to the Release 2019.04 milestone Feb 25, 2019
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Feb 25, 2019

Thank you for the review.

@cladmi cladmi deleted the pr/bug/export/term_variables branch February 25, 2019 17:06
@danpetry danpetry modified the milestone: Release 2019.04 Mar 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aabadie aabadie Awaiting requested review from aabadie

@jcarrano jcarrano Awaiting requested review from jcarrano

Assignees

No one assigned

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2019.04

Development

Successfully merging this pull request may close these issues.

3 participants

@cladmi @jcarrano @danpetry