Skip to content

boards/native: remove toolchain variables duplication#11627

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/native/toolchain_duplication
Jun 4, 2019
Merged

boards/native: remove toolchain variables duplication#11627
miri64 merged 1 commit intoRIOT-OS:masterfrom
cladmi:pr/native/toolchain_duplication

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Jun 4, 2019

Contribution description

boards/native: remove toolchain variables duplication

The toolchain variables were overwritten anyway in
makefiles/toolchain/gnu|llvm.inc.mk as they use = affectation,
except for OBJCOPY.

On my Linux ubuntu bionic machine, the only difference for info-build
with both gnu/llvm and also with LTO=1
is that it now uses an absolute path for objcopy

-OBJCOPY: objcopy
+OBJCOPY: /usr/bin/objcopy

On the OSX 17.7.0 I tested, there was no difference.

Testing procedure

Compare the output of info-build with different toolchains and enabling LTO.

I used this output:

{ TOOLCHAIN=gnu BOARD=native make -C examples/hello-world/ info-build; TOOLCHAIN=gnu LTO=1 BOARD=native make -C examples/hello-world/ info-build; TOOLCHAIN=llvm BOARD=native make -C examples/hello-world/ info-build; TOOLCHAIN=llvm LTO=1 BOARD=native make -C examples/hello-world/ info-build; } | tee info_build_pr

On my Linux machine I get this difference

diff -u info_build_*
--- info_build_master   2019-06-04 15:43:51.031958413 +0200
+++ info_build_pr       2019-06-04 15:44:01.800045718 +0200
@@ -88,7 +88,7 @@
        -Wl,--gc-sections
        -ffunction-sections

-OBJCOPY: objcopy
+OBJCOPY: /usr/bin/objcopy
 OFLAGS:

 FLASHER: true
@@ -241,7 +241,7 @@
        -ffunction-sections
        -flto

-OBJCOPY: objcopy
+OBJCOPY: /usr/bin/objcopy
 OFLAGS:

 FLASHER: true
@@ -390,7 +390,7 @@
        -Wl,--gc-sections
        -ffunction-sections

-OBJCOPY: objcopy
+OBJCOPY: /usr/bin/objcopy
 OFLAGS:

 FLASHER: true
@@ -540,7 +540,7 @@
        -ffunction-sections
        -flto

-OBJCOPY: objcopy
+OBJCOPY: /usr/bin/objcopy
 OFLAGS:

 FLASHER: true

On OSX I get the same output.

Issues/PRs references

Found as a dependency for cleaning exports for compilation variables #10850
Board/cpu cleanup #8713 (comment)

@cladmi
boards/native: remove toolchain variables duplication
9c5688e 
The toolchain variables were overwritten anyway in
`makefiles/toolchain/gnu|llvm.inc.mk` as they use `=` affectation,
except for `OBJCOPY`.

On my Linux ubuntu bionic machine, the only difference for `info-build`
with both gnu/llvm and also with LTO=1
is that it now uses an absolute path for `objcopy`

    -OBJCOPY: objcopy
    +OBJCOPY: /usr/bin/objcopy

On the OSX 17.7.0 I tested, there was no difference.
@cladmi cladmi added Platform: native Platform: This PR/issue effects the native platform Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … labels Jun 4, 2019
@cladmi cladmi added this to the Release 2019.07 milestone Jun 4, 2019
@cladmi cladmi requested review from jnohlgard and miri64 June 4, 2019 14:01
@cladmi cladmi added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 4, 2019
miri64
miri64 reviewed Jun 4, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Confirmed that this is a possible duplication. I'd still like to (have this) test(ed) on an Arch machine.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jun 4, 2019

Both gnu and llvm still work on Ubuntu 16.04.

miri64
miri64 approved these changes Jun 4, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Also tested on Arch. Go.

@miri64 miri64 merged commit 413c6d1 into RIOT-OS:master Jun 4, 2019
@cladmi cladmi mentioned this pull request Jun 4, 2019
Open
10 tasks
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Jun 4, 2019

Thanks for the review.

@cladmi cladmi deleted the pr/native/toolchain_duplication branch June 4, 2019 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 approved these changes

@jnohlgard jnohlgard Awaiting requested review from jnohlgard

Assignees

No one assigned

Labels

Area: toolchain Area: toolchains; everything related to compilation, libc, linking, … CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: native Platform: This PR/issue effects the native platform Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

Release 2019.07

Development

Successfully merging this pull request may close these issues.

2 participants

@cladmi @miri64