Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

posix: consolidate include paths #3611

Merged
merged 1 commit into from
Sep 11, 2015
Merged

posix: consolidate include paths #3611

merged 1 commit into from
Sep 11, 2015

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Aug 11, 2015

Though I introduced this split myself, I see no specific reason for doing it this way and find it rather tidious. Note that we can't include the POSIX headers into sys/include, since otherwise POSIX applications would not build on native.

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: POSIX Area: POSIX API wrapper Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. labels Aug 11, 2015
@miri64 miri64 added this to the Release 2015.08 milestone Aug 11, 2015
@miri64 miri64 force-pushed the posix/api/consolidate-include-paths branch from bf577cc to 45c0681 Compare August 11, 2015 18:04
This was referenced Aug 11, 2015
Merged
Merged
@miri64 miri64 force-pushed the posix/api/consolidate-include-paths branch from 45c0681 to 7958314 Compare August 26, 2015 11:56
@miri64
Copy link
Member Author

miri64 commented Aug 26, 2015

Rebased to current master

Kijewski
Kijewski reviewed Aug 26, 2015
View reviewed changes
pkg/libcoap/0001-Add-RIOT-Makefile.patch
@@ -15,7 +15,7 @@ index 0000000..f90baa1
+++ b/Makefile
@@ -0,0 +1,5 @@
+MODULE:=$(shell basename $(CURDIR))
+INCLUDES += -I$(RIOTBASE) -I$(RIOTBASE)/sys/include -I$(RIOTBASE)/core/include -I$(RIOTBASE)/drivers/include -I$(RIOTBASE)/drivers/cc110x_legacy/include -I$(RIOTBASE)/cpu/arm7_common/include -I$(RIOTBASE)/sys/net/transport_layer/include -I$(RIOTBASE)/sys/net/sixlowpan/include/ -I$(RIOTBASE)/sys/net/ieee802154/include -I$(RIOTBASE)/sys/net/net_help -I$(RIOTBASE)/sys/posix/include -I$(RIOTBASE)/sys/posix/pnet/include
+INCLUDES += -I$(RIOTBASE) -I$(RIOTBASE)/sys/include -I$(RIOTBASE)/core/include -I$(RIOTBASE)/drivers/include -I$(RIOTBASE)/drivers/cc110x_legacy/include -I$(RIOTBASE)/cpu/arm7_common/include -I$(RIOTBASE)/sys/net/transport_layer/include -I$(RIOTBASE)/sys/net/sixlowpan/include/ -I$(RIOTBASE)/sys/net/ieee802154/include -I$(RIOTBASE)/sys/net/net_help -I$(RIOTBASE)/sys/posix/include
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should not be needed at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed.

@Kijewski Kijewski 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 Aug 29, 2015
@Kijewski
Copy link
Contributor

Two errors for native: https://travis-ci.org/RIOT-OS/RIOT/jobs/77329976#L1179

@miri64
Copy link
Member Author

miri64 commented Aug 30, 2015

Then this PR is not mergable until uart0 for native is dropped T.T. (Those apps include shell, for native this draws in uart0 and thus posix, causing the include path for dev_eth_tap.h to look into the posix path, breaking the normal system includes there)

@miri64
Copy link
Member Author

miri64 commented Aug 30, 2015

Alternatively we could move socket_base to sys/include for now (will be removed after #3615 got merged and the modules using socket_base are adapted anyways).

@miri64
Copy link
Member Author

miri64 commented Aug 30, 2015

Adapted #3611 (comment). Scream if you don't like that. [edit: uart0 was removed from master so I removed that change. See https://github.com//pull/3611#issuecomment-138316969]

@miri64 miri64 force-pushed the posix/api/consolidate-include-paths branch from 07c2c1f to a4905b7 Compare September 7, 2015 14:47
@miri64
Copy link
Member Author

miri64 commented Sep 7, 2015

Rebased to current master and removed 07c2c1f as uart0 was dropped from master.

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 7, 2015
@miri64
Copy link
Member Author

miri64 commented Sep 7, 2015

Travis is happy now.

@OlegHahm OlegHahm assigned A-Paul and cgundogan and unassigned Kijewski and A-Paul Sep 11, 2015
@cgundogan
Copy link
Member

ACK. building for native and iotlab-m3 works without error. I added USEPKG += libcoap to test this to the gnrc_networking example

miri64 added a commit that referenced this pull request Sep 11, 2015
@miri64
Merge pull request #3611 from authmillenon/posix/api/consolidate-incl…
3079b30 
…ude-paths

posix: consolidate include paths
@miri64 miri64 merged commit 3079b30 into RIOT-OS:master Sep 11, 2015
@miri64 miri64 deleted the posix/api/consolidate-include-paths branch September 11, 2015 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cgundogan cgundogan

Labels
Area: POSIX Area: POSIX API wrapper CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2015.09
Development

Successfully merging this pull request may close these issues.
4 participants
@miri64 @Kijewski @cgundogan @A-Paul