Skip to content

native: fix warning error OS X#3393

Merged
thomaseichinger merged 9 commits intoRIOT-OS:masterfrom
thomaseichinger:pr/fix_osx_warnings_errors
Sep 3, 2015
Merged

native: fix warning error OS X#3393
thomaseichinger merged 9 commits intoRIOT-OS:masterfrom
thomaseichinger:pr/fix_osx_warnings_errors

Conversation

@thomaseichinger
Copy link
Copy Markdown
Member

@thomaseichinger thomaseichinger commented Jul 14, 2015

rebased upon #3290 and #3390 to build again after merge of #3319.
@emmanuelsearch can you verify?

@thomaseichinger thomaseichinger 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 Impact: major The PR changes a significant part of the code base. It should be reviewed carefully OS: Mac OS X Host OS: This PR/issue concerns usage of RIOT with Mac OS X as a host system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Jul 14, 2015
@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Jul 14, 2015

@thomaseichinger could you add comments to the #else and #endif to the places where they are more than only 2-3 lines apart?

@emmanuelsearch
Copy link
Copy Markdown
Member

emmanuelsearch commented Jul 14, 2015

Works for me! (no warnings anymore when I compile on my machine with OS X 10.10.3)

@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Jul 14, 2015

@thomaseichinger please squash

@OlegHahm OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jul 15, 2015
@thomaseichinger thomaseichinger added the State: waiting for other PR State: The PR requires another PR to be merged first label Jul 22, 2015
@thomaseichinger thomaseichinger force-pushed the pr/fix_osx_warnings_errors branch 4 times, most recently from 9132c65 to e92debe Compare July 23, 2015 08:27
@thomaseichinger
Copy link
Copy Markdown
Member Author

thomaseichinger commented Jul 23, 2015

Updated after merge of #3390 and squashed.

@thomaseichinger thomaseichinger removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jul 23, 2015
@thomaseichinger thomaseichinger mentioned this pull request Aug 3, 2015
Closed
@thomaseichinger thomaseichinger added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Aug 3, 2015
@thomaseichinger thomaseichinger force-pushed the pr/fix_osx_warnings_errors branch from e92debe to 7ca53dd Compare August 19, 2015 23:34
@thomaseichinger
Copy link
Copy Markdown
Member Author

thomaseichinger commented Aug 19, 2015

@emmanuelsearch ping

@emmanuelsearch
Copy link
Copy Markdown
Member

emmanuelsearch commented Aug 25, 2015

No warning for me either when I build with 10.10.5. However, if I run the Default example I get the following runtime error:

kernel_init(): This is RIOT! (Version: 2014.12-2274-g066fe-pr-3393)
kernel_init(): jumping into first task...
UART0 thread started.
uart0_init() [OK]
Native LTC4150 initialized.
No hwtimer left.
/Users/emmanuelB/Documents/work/CODING/RIOT/RIOT/examples/default/bin/native/default.elf: ltc4150_enable_int: hwtimer_set
make: *** [term] Error 1

@thomaseichinger thomaseichinger mentioned this pull request Aug 27, 2015
Closed
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Aug 27, 2015

@emmanuelsearch Hmm, ltc4150 has been kicked from native a while ago.

@thomaseichinger Maybe this PR just needs a rebase?

@thomaseichinger thomaseichinger force-pushed the pr/fix_osx_warnings_errors branch from 7ca53dd to 6830c5d Compare August 27, 2015 13:22
@thomaseichinger
Copy link
Copy Markdown
Member Author

thomaseichinger commented Aug 27, 2015

rebased. @emmanuelsearch could you try to run default again?

@thomaseichinger
Copy link
Copy Markdown
Member Author

thomaseichinger commented Aug 27, 2015

OK, rebasing introduced a new problem. You should see something like:

ld: file too small (length=8) file '/Users/thomas/code/RIOT/examples/gnrc_networking/bin/native/gnrc.a' for architecture i386
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@thomaseichinger
Copy link
Copy Markdown
Member Author

thomaseichinger commented Aug 27, 2015

The problem is that an empty archive gnrc.a gets created but there are never files added to it. Executing the linker command manually without the mentioned archive succeeds and results in a working binary.

@thomaseichinger
Copy link
Copy Markdown
Member Author

thomaseichinger commented Aug 27, 2015

@emmanuelsearch I added a workaround for the empty archive problem. Could you test again?

@emmanuelsearch
Copy link
Copy Markdown
Member

emmanuelsearch commented Aug 27, 2015

great. I can now run gnrc_networking. Have not tested more than the shell though. As discussed offline, I suspect we still have tap setup issues, right?

@emmanuelsearch
Copy link
Copy Markdown
Member

emmanuelsearch commented Aug 27, 2015

Assuming tap setup issues are a separate issue, I'd ACK this ;)

@thomaseichinger thomaseichinger force-pushed the pr/fix_osx_warnings_errors branch from 7fc44af to 8483d31 Compare September 1, 2015 15:37
@thomaseichinger thomaseichinger added this to the Release 2015.09 milestone Sep 2, 2015
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Sep 2, 2015

please rebase... I'm testing this now.

@kaspar030 kaspar030 assigned kaspar030 and unassigned x3ro Sep 2, 2015
@thomaseichinger thomaseichinger force-pushed the pr/fix_osx_warnings_errors branch from 8483d31 to 425a3ef Compare September 2, 2015 15:36
@thomaseichinger
Copy link
Copy Markdown
Member Author

thomaseichinger commented Sep 2, 2015

rebased

@emmanuelsearch
Copy link
Copy Markdown
Member

emmanuelsearch commented Sep 3, 2015

I tried it on OS X 10.10.5. It compiles without warning and runs fine: I get a functioning shell with default and gnrc_networking.

@emmanuelsearch
Copy link
Copy Markdown
Member

emmanuelsearch commented Sep 3, 2015

ACK.

@emmanuelsearch
Copy link
Copy Markdown
Member

emmanuelsearch commented Sep 3, 2015

(once Travis is green ;)

Kijewski
Kijewski reviewed Sep 3, 2015
View reviewed changes
Makefile.include Outdated
Copy link
Copy Markdown
Contributor

@Kijewski Kijewski Sep 3, 2015

Choose a reason for hiding this comment

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

Please try

$(shell find $(BASELIBS) -size +8c)

which should return the .a files that are bigger than eight bytes. The .a header is 8 bytes.

Copy link
Copy Markdown
Member Author

@thomaseichinger thomaseichinger Sep 3, 2015

Choose a reason for hiding this comment

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

This leaves me with a linker command without any archives when I call it with make clean all. The archives cannot be found by find as they are removed before. Issuing a consecutive make all succeeds then.

Copy link
Copy Markdown
Contributor

@Kijewski Kijewski Sep 3, 2015

Choose a reason for hiding this comment

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

Ah right, $$(find $(BASELIBS) -size +8c) then? :)

Copy link
Copy Markdown
Member Author

@thomaseichinger thomaseichinger Sep 3, 2015

Choose a reason for hiding this comment

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

This works! Thanks a lot! Updated the commit.

@thomaseichinger thomaseichinger force-pushed the pr/fix_osx_warnings_errors branch from 425a3ef to 9882f18 Compare September 3, 2015 09:50
@thomaseichinger
native: exclude emtpy archives from linking on OSX
24e9de6 
OS X linker won't link archives consisting of the archives header only.
@thomaseichinger thomaseichinger force-pushed the pr/fix_osx_warnings_errors branch from 9882f18 to 24e9de6 Compare September 3, 2015 09:56
@thomaseichinger
Copy link
Copy Markdown
Member Author

thomaseichinger commented Sep 3, 2015

Travis passed. Go

thomaseichinger added a commit that referenced this pull request Sep 3, 2015
@thomaseichinger
Merge pull request #3393 from thomaseichinger/pr/fix_osx_warnings_errors
fa11195 
native: fix warning error OS X
@thomaseichinger thomaseichinger merged commit fa11195 into RIOT-OS:master Sep 3, 2015
@thomaseichinger thomaseichinger mentioned this pull request Sep 3, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@kaspar030 kaspar030

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: major The PR changes a significant part of the code base. It should be reviewed carefully 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) Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

Release 2015.09

Development

Successfully merging this pull request may close these issues.

7 participants

@thomaseichinger @jnohlgard @emmanuelsearch @kaspar030 @Kijewski @x3ro @OlegHahm