Skip to content

Conversation

@mario-vera
Copy link
Contributor

Expand Linux platforms supported under DEPLOYMENT_RUNTIME_SWIFT=0 umbrella.

#include <libxml/xpath.h>
#include <libxml/xpathInternals.h>
#include <libxml/dict.h>
#include "CFInternal.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

So we don't use CFInternal.h here at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't removed, it moved up & uses qualified path. Check line 15: <CoreFoundation/CFInternal.h>

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks. Misread the diff.


saxHandler->initialized = XML_SAX2_MAGIC; // make sure start/endElementNS are used
#else
memset(&saxHandler, 0, sizeof(saxHandler));
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't the calloc above already promise us zeroed memory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, this line is not needed, I'll remove it.

lib/script.py Outdated
#script += subitem
continue
else:
script += item
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain a bit more about what this is doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please add Lily to review this one. script is a string. items is a list containing strings and lists. On python 2.7 we can not append a list into a string, so we need to walk it, appending the strings on the array.
The comment is to highlight the fact that I found no usage for the nested lists... Not sure what the intended usage was.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC: This path is triggered by the += [ … ] command that adds the block runtime, which is invoked if libdispatch is not available. Can you just build libdispatch as part of your CF build instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can also fix that command to add single items instead of a list.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, for context: this is the portion of build.py that is triggering that behavior:

# This code is already in libdispatch so is only needed if libdispatch is
# NOT being used
if "LIBDISPATCH_SOURCE_DIR" not in Configuration.current.variables:
    sources += (['closure/data.c', 'closure/runtime.c'])

It'd be nice to fix this bit, but in general you want to set your LIBDISPATCH_SOURCE_DIR environment variable correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applying LIBDISPATCH_SOURCE_DIR did not work for me. I just auggested a more pythonic way of applying the product commands... hopefully that is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, I'm compiling from within CoreFoundation, so the build.py I'm using is in CoreFoundation/build.py, that scripts do not have any references to LIBDISPATCH_SOURCE_DIR.

@millenomi
Copy link
Contributor

@swift-ci please test

@millenomi
Copy link
Contributor

@swift-ci please test and merge

@swift-ci swift-ci merged commit f581005 into swiftlang:master May 30, 2018
spevans added a commit to spevans/swift-corelibs-foundation that referenced this pull request Jun 2, 2018
- This required by swiftlang#1559 but it was only tested on Linux.
spevans added a commit to spevans/swift-corelibs-foundation that referenced this pull request Jun 2, 2018
spevans added a commit to spevans/swift-corelibs-foundation that referenced this pull request Jun 4, 2018
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.

4 participants