-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fixed for Cygwin #733
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
Fixed for Cygwin #733
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -165,6 +165,10 @@ public final class ManifestLoader: ManifestLoaderProtocol { | |
var cmd = [resources.swiftCompilerPath.asString] | ||
cmd += ["--driver-mode=swift"] | ||
cmd += verbosity.ccArgs | ||
#if CYGWIN | ||
cmd += ["-D", "CYGWIN"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this used in the current patch? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is necessary to distinguish Cygwin from other Windows variants. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code is loading the manifest though, and we don't want users to conditionalize their manifests in this way. Since it isn't currently required, I recommend we avoid it here. |
||
cmd += ["-I", "/usr/include"] | ||
#endif | ||
cmd += ["-I", resources.libraryPath.asString] | ||
|
||
// When running from Xcode, load PackageDescription.framework | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,6 +116,8 @@ if platform.system() == 'Darwin': | |
|
||
if platform.system() == 'Linux': | ||
g_shared_lib_ext = ".so" | ||
elif platform.system().startswith('CYGWIN'): | ||
g_shared_lib_ext = ".dll" | ||
else: | ||
g_shared_lib_ext = ".dylib" | ||
|
||
|
@@ -321,10 +323,10 @@ def parse_manifest(): | |
# We have a *very* strict format for our manifest to make parsing more | ||
# robust. We use this to determine the target and product definitions. | ||
target_pattern = re.compile( | ||
r'Target\(.*?name: "(.*?)",\n *dependencies: (\[.*?\])\)', | ||
r'Target\(.*?name: "(.*?)",\r?\n? *dependencies: (\[.*?\])\)', | ||
re.DOTALL | re.MULTILINE) | ||
product_pattern = re.compile( | ||
r'Library\([\n ]*name: \"(.*?)\",[\n ]*type: (.*?),[\n ]*targets: (\[.*?\])[\n ]*\)', | ||
r'Library\([\r\n ]*name: \"(.*?)\",[\r\n ]*type: (.*?),[\r\n ]*targets: (\[.*?\])[\r\n ]*\)', | ||
re.DOTALL | re.MULTILINE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should need this, there should be a way to open the manifest file such that Python will automatically do the line ending conversion on reading the data. |
||
|
||
# Load the manifest as a string (we always assume UTF-8 encoding). | ||
|
@@ -560,6 +562,8 @@ def create_bootstrap_files(sandbox_path, args): | |
link_command.extend(["-L", args.foundation_path]) | ||
if args.libdispatch_build_dir: | ||
link_command.extend(["-L", os.path.join(args.libdispatch_build_dir, "src", ".libs")]) | ||
if not target.is_library and platform.system().startswith("CYGWIN"): | ||
link_command.extend(["-Xlinker", "--allow-multiple-definition"]) | ||
|
||
# Write out the link command. | ||
if target.is_library: | ||
|
@@ -644,12 +648,21 @@ def process_runtime_libraries(build_path, args, bootstrap=False): | |
# We include an RPATH entry so that the Swift libraries can be found | ||
# relative to where it will be installed (in 'lib/swift/pm/...'). | ||
runtime_lib_path = os.path.join(lib_path, "libPackageDescription.so") | ||
|
||
# Change shared object extension for Cygwin | ||
if platform.system().startswith("CYGWIN"): | ||
runtime_lib_path = os.path.join(lib_path, "libPackageDescription.dll") | ||
|
||
cmd = [args.swiftc_path, "-Xlinker", "-shared", "-o", runtime_lib_path, | ||
"-Xlinker", "--whole-archive", | ||
"-Xlinker", input_lib_path, | ||
"-Xlinker", "--no-whole-archive", "-lswiftGlibc", | ||
"-Xlinker", "-rpath=$ORIGIN/../linux"] | ||
|
||
# Add link flags for Cygwin | ||
if platform.system().startswith("CYGWIN"): | ||
cmd.extend(["-Xlinker", "--allow-multiple-definition"]) | ||
|
||
# We need to pass one swift file here to bypass the "no input files" | ||
# error. | ||
tf = tempfile.NamedTemporaryFile(suffix=".swift") | ||
|
@@ -895,6 +908,8 @@ def main(): | |
# libraries. | ||
if platform.system() == 'Linux': | ||
embed_rpath = "$ORIGIN/../lib/swift/linux" | ||
elif platform.system().startswith('CYGWIN'): | ||
embed_rpath = "$ORIGIN/../lib/swift/cygwin" | ||
else: | ||
embed_rpath = "@executable_path/../lib/swift/macosx" | ||
build_flags.extend(["-Xlinker", "-rpath", "-Xlinker", embed_rpath]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Swift itself not define an
os
condition for Cygwin?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't use it.
os(Cygwin)
PR (swiftlang/swift#2351) was not accepted for Swift compiler.There were some opinions that Cygwin is only an environment and not OS, and
os(Windows)
is true for all Windows variants.We can not distinguish the Cygwin from other Windows variant, MSVC, MinGW.
os(Windows)
and CYGWIN macro will be evaluated to true for Cygwin environment.The CYGWIN macro is also used in Swift standard library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it... I hope eventually there is some builtin way to identify this.