Skip to content

Conversation

smlng
Copy link
Member

@smlng smlng commented Sep 1, 2016

longer discussion here #5787, corrects some changes introduces in #5742 for OSX

@smlng smlng force-pushed the pr/fix_osx_5742 branch 2 times, most recently from 8c7fcb9 to 9956790 Compare September 1, 2016 08:19
@jnohlgard
Copy link
Member

ffunction-sections and fdata-sections are already set and -flto is set when the environment variable LTO=1

@smlng
Copy link
Member Author

smlng commented Sep 1, 2016

@gebart you're right with -flto, I'll correct that. But did you read this, it states that -ffunction-sections should be added to linker flags, too - not only CFLAGS. Or are CFLAGS applied to the linker as well?

@jnohlgard
Copy link
Member

CFLAGS should be added to link flags. I think it was added some time ago, but I'm replying from the phone so I can't look up the source now.

@smlng
Copy link
Member Author

smlng commented Sep 1, 2016

From what I found, CFLAGS are (somehow) added to LINKFLAGS for many boards, but not in native. But, as already said I'm not expert on compiler or linker options, plus not familiar with the internal details of RIOTs build system - maybe you can check on this later on, when you have a bigger screen and keyboard available?

@kYc0o kYc0o added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: native Platform: This PR/issue effects the native platform Area: build system Area: Build system OS: Mac OS X Host OS: This PR/issue concerns usage of RIOT with Mac OS X as a host system labels Sep 2, 2016
@kYc0o kYc0o modified the milestones: FIX ME FIRST, Release 2016.10 Sep 2, 2016
@smlng
Copy link
Member Author

smlng commented Sep 6, 2016

Ping! This isn't that hard, is it? Could we resolve it (very) soon, its an annoying bug when testing on macOS.

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 6, 2016
@jnohlgard
Copy link
Member

I'm sorry, I will not be able to assist here for a few days. To anyone interested in this issue, feel free to dig through the makefiles, I suggest starting with git grep LINKFLAGS

@smlng
Copy link
Member Author

smlng commented Sep 6, 2016

That's what I did and - as said above - I found that for some boards CFLAGS are added to LINKFLAGS, but not for native, which this patch corrects.

@thomaseichinger
Copy link
Member

This change definitely fixes building on Darwin systems.
CC: @gebart @kYc0o

@basilfx
Copy link
Member

basilfx commented Sep 6, 2016

I can also confirm this works.

@smlng
Copy link
Member Author

smlng commented Sep 12, 2016

mhm, everybody seems happy - even Murdock, though this doesn't count in this case as it is OS X specific. Anyhow, I'll squash the 2 commits and then ACK + GO.

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 12, 2016
  - corrects and enhances changes introduced in RIOT-OS#5742
@smlng
Copy link
Member Author

smlng commented Sep 12, 2016

I'm reluctant to merge my own PR, I just tested it on OS X and Linux again: works for both. We can wait for Murdock to complete and then GO?

@smlng smlng added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 12, 2016
@smlng
Copy link
Member Author

smlng commented Sep 13, 2016

Does anybody understand why Murdock fails building examples/arduino_hello-world, I don't see how this should be related to the failure?!

@jnohlgard
Copy link
Member

I believe the arduino error is unrelated to this PR and seem to be a race condition caused by using the application source directory as the destination for the _sketches.cpp file.

@jnohlgard
Copy link
Member

The failing build is not related to this PR. The change looks OK and is confirmed working by maintainers above. ACK & go

@jnohlgard jnohlgard merged commit ee486fd into RIOT-OS:master Sep 16, 2016
@smlng smlng deleted the pr/fix_osx_5742 branch September 27, 2016 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 OS: Mac OS X Host OS: This PR/issue concerns usage of RIOT with Mac OS X as a host system Platform: native Platform: This PR/issue effects the native platform Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants