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.

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

SR-3658 – Fix error in importing Foundation #843

Merged
merged 1 commit into from
Jan 27, 2017

Conversation

swizzlr
Copy link
Contributor

@swizzlr swizzlr commented Jan 27, 2017

SR-3658 – Fix error in importing Foundation under Linux. This commit may resolve the described error.

Honestly, I have no idea if this'll work. My machine does not have the horsepower to compile this project, but at the very least maybe the CI can take a crack at it. From a cursory review this line seems unnecessary.

SR-3658 – Fix error in importing Foundation under Linux. This commit may resolve the described error.
@parkera
Copy link
Member

parkera commented Jan 27, 2017

We do need Block.h for CF itself, but we may not need it to be in a header that is part of the module map. Basically - anywhere we call Block_copy and friends will still need this include.

@e78l
Copy link
Contributor

e78l commented Jan 27, 2017

From grep, Block.h seems to be included in these files:
./CoreFoundation/Base.subproj/CFBase.h:72:#include <Block.h>
./CoreFoundation/Base.subproj/CFUtilities.c:57:#include <Block.h>
./CoreFoundation/Collections.subproj/CFBasicHash.c:14:#include <Block.h>
./CoreFoundation/RunLoop.subproj/CFRunLoop.c:106:#include <Block.h>

I tried adding to build.py:
'-I./closure',

But then xctest had a problem too, so I just copied Block.h into an existing include path.

@swizzlr
Copy link
Contributor Author

swizzlr commented Jan 27, 2017

Thing is, this was working in 3.0 - it was a commit migrating the Sierra source dump that introduced the offending code - but CF has had blocks for years now?

Or is it the case that CFBase, by including Blocks.h in the header, and not source files, is trying to make Swift eventually include Blocks.h, which isn't used under Linux?

@parkera
Copy link
Member

parkera commented Jan 27, 2017

Let's try this: it'll tell us where we need the include for real.

@parkera
Copy link
Member

parkera commented Jan 27, 2017

@swift-ci please test

@parkera
Copy link
Member

parkera commented Jan 27, 2017

What's interesting is that the include has been in the Darwin CFBase.h for quite some time; so it's surprising that it was introduced here with the Sierra merge.

@swizzlr
Copy link
Contributor Author

swizzlr commented Jan 27, 2017

Jesus.

@parkera
Copy link
Member

parkera commented Jan 27, 2017

I think it was probably removed for this very reason in our original CF import into this project, but then put back with the rest of the Sierra fixes. We'll need to make sure it stays out in the future.

@parkera
Copy link
Member

parkera commented Jan 27, 2017

@swizzlr will you put a PR up for the swift-3.1 branch with this change?

@parkera parkera merged commit c53075d into apple:swift-3.1-branch Jan 27, 2017
@parkera
Copy link
Member

parkera commented Jan 27, 2017

Sorry, I mean the master branch.

@swizzlr swizzlr deleted the patch-1 branch January 29, 2017 18:28
@thawkins
Copy link

thawkins commented Feb 2, 2017

Can we please get this fix merged into the master, the master wont build at the moment because of this.

@swizzlr
Copy link
Contributor Author

swizzlr commented Feb 2, 2017

Wait, don't we have build snapshots for Swift 3.1?

@thawkins
Copy link

thawkins commented Feb 2, 2017 via email

@swizzlr swizzlr restored the patch-1 branch February 2, 2017 14:47
@thawkins
Copy link

thawkins commented Feb 3, 2017

folks im still seeing this on the swift-3.1-branch

../build/buildbot_linux/foundation-linux-x86_64/Foundation/usr//lib/swift/CoreFoundation/ForSwiftFoundationOnly.h:265:60: note: insert '_Nonnull' if the array parameter should never be null
CF_EXPORT void _cf_uuid_unparse_upper(const _cf_uuid_t uu, _cf_uuid_string_t out);
^
_Nonnull
CoreFoundation/Collections.subproj/CFBasicHash.c:14:10: fatal error: 'Block.h' file not found
#include <Block.h>
^
18 warnings and 1 error generated.

@thawkins
Copy link

thawkins commented Feb 3, 2017

In order to get it to compile i had to insert the following between update-checkout and build-script

sed -i '/#include <Block.h>/d' swift-corelibs-foundation/CoreFoundation/Base.subproj/CFBase.h
sed -i 's/#include <Block.h>/#include <closure/Block.h>/g' swift-corelibs-foundation/CoreFoundation/Collections.subproj/CFBasicHash.c
sed -i 's/#include <Block.h>/#include <closure/Block.h>/g' swift-corelibs-foundation/CoreFoundation/RunLoop.subproj/CFRunLoop.c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants