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.

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

common: Remove redundant includes - 4 #15251

Merged
merged 2 commits into from Jun 16, 2017

Conversation

Projects
None yet
4 participants
@joscollin
Member

joscollin commented May 24, 2017

Fixes: http://tracker.ceph.com/issues/19883 (Partially)

Signed-off-by: Jos Collin jcollin@redhat.com

common: Remove redundant includes
Fixes: http://tracker.ceph.com/issues/19883 (Partially)

Signed-off-by: Jos Collin <jcollin@redhat.com>
@liewegas

This comment has been minimized.

Member

liewegas commented May 30, 2017

@wjwithagen this and #15267 test out fine. should we continue to merge these or is there some check we should do first to avoid breakage?

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented May 30, 2017

@liewegas @joscollin @yuriw
From visual inspection I certainly have the feeling that this PR is going to break FreeBSD building.
I do see some includes that I have added because FreeBSD needed.
And for the ones that were compatible with Linux I chose to just add them, and not #ifdef them for FreeBSD. THings that spring to my mind are some of the includes with the DNS files, and removing pthread is probably also a bad idea.

BUT I really need to test this. So I'll have to make some diffs, and apply to a testdir, and see what comes of it. That might take me a few days to figure out what the minimal set is that FreeBSD needs to keep.

Note that I like the cleanup from a software maintenance point , but I would not expect any measurable decrease in build time. Even more soe if you're using ccache.

@badone badone requested a review from wjwithagen May 31, 2017

@joscollin

This comment has been minimized.

Member

joscollin commented May 31, 2017

@liewegas @wjwithagen @yuriw pthread is included via #include "include/Context.h"and that's the reason it doesn't report error for me and in the Jenkins too. So as @wjwithagen said, this needs testing in FreeBSD to make sure if there is something to be reverted.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented May 31, 2017

@joscollin @liewegas
Like I thought, reorg is a bit too agresive for FreeBSD.

In file included from /home/jenkins/workspace/ceph-master/src/common/dns_resolve.h:17:
/usr/include/resolv.h:157:3: error: field has incomplete type 'struct sockaddr_in'
                nsaddr_list[MAXNS];     /*%< address of name server */
                ^
/usr/include/resolv.h:156:9: note: forward declaration of 'sockaddr_in'
        struct sockaddr_in
               ^
/usr/include/resolv.h:196:21: error: field has incomplete type 'struct sockaddr_in'
        struct sockaddr_in      sin;
                                ^
/usr/include/resolv.h:156:9: note: forward declaration of 'sockaddr_in'
        struct sockaddr_in
               ^
2 errors generated.

So this set I need to figure out.

common: Remove redundant includes
Fixed: error: field has incomplete type 'struct sockaddr_in'

Signed-off-by: Jos Collin <jcollin@redhat.com>
@joscollin

This comment has been minimized.

Member

joscollin commented Jun 10, 2017

@wjwithagen Fixed, please check in FreeBSD now and let me know. Thank you for the testing.

@wjwithagen

Compiles and rund oke on FreeBSD

@joscollin

This comment has been minimized.

Member

joscollin commented Jun 11, 2017

Jenkins Retest this please

1 similar comment
@joscollin

This comment has been minimized.

Member

joscollin commented Jun 13, 2017

Jenkins Retest this please

@joscollin

This comment has been minimized.

Member

joscollin commented Jun 13, 2017

@liewegas You can add your testing label now.

@liewegas liewegas merged commit 94d3324 into ceph:master Jun 16, 2017

3 of 4 checks passed

arm64 make check arm64 make check started
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details

@joscollin joscollin deleted the joscollin:wip-cleanup-redundant-headers-4 branch Jun 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment