Skip to content

Conversation

@compnerd
Copy link
Member

llbuild depends on LLVMSupport and LLVMDemangle. These libraries
require that LLVM_ON_WIN32 is defined when building them on Windows
(and technically LLVM_ON_UNIX on Unicies). This is required when
building llbuild with swift-package-manager on Windows.

@compnerd
Copy link
Member Author

CC: @tomerd @neonichu @abertelrud

@compnerd
Copy link
Member Author

@swift-ci please test

@tomerd
Copy link

tomerd commented Aug 12, 2020

seems reasonable to me, do we need to do the same for LLVM_ON_UNIX as you suggest?

@compnerd
Copy link
Member Author

Yeah, I think that explicitly defining LLVM_ON_UNIX is nicer, but that should be a separate change.

llbuild depends on LLVMSupport and LLVMDemangle.  These libraries
require that `LLVM_ON_WIN32` is defined when building them on Windows
(and technically `LLVM_ON_UNIX` on Unicies).  This is required when
building llbuild with swift-package-manager on Windows.
@compnerd
Copy link
Member Author

@swift-ci please test

1 similar comment
@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

@swift-ci please smoke test Linux platform

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/swift-package-manager#2869

@swift-ci please smoke test

@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/swift-package-manager#2869

@swift-ci please test

@tomerd
Copy link

tomerd commented Aug 18, 2020

lgtm, @ddunbar do you normally merge PRs here or someone else on your team?

@ddunbar
Copy link
Contributor

ddunbar commented Aug 18, 2020

@dmbryson usually watches stuff these days

@tomerd
Copy link

tomerd commented Aug 18, 2020

@dmbryson can you help @compnerd gets this merged

@dmbryson dmbryson merged commit 77a482e into swiftlang:master Aug 18, 2020
@aciidgh
Copy link
Contributor

aciidgh commented Aug 19, 2020

Unfortunately, this breaks compiling llbuild as a package with SwiftPM 5.2 because of some bug with conditionals:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   swift-build                   	0x0000000107148048 Swift runtime failure: Unexpectedly found nil while unwrapping an Optional value + 0 (PackageBuilder.swift:837) [inlined]
1   swift-build                   	0x0000000107148048 closure #2 in PackageBuilder.buildConditions(from:) + 0 (PackageBuilder.swift:837) [inlined]
2   swift-build                   	0x0000000107148048 thunk for @callee_guaranteed (@guaranteed String) -> (@owned Platform, @error @owned Error) + 0 [inlined]
3   swift-build                   	0x0000000107148048 specialized Collection.map<A>(_:) + 0 [inlined]
4   swift-build                   	0x0000000107148048 specialized PackageBuilder.buildConditions(from:) + 1672 (PackageBuilder.swift:837)
5   swift-build                   	0x0000000107141f72 PackageBuilder.buildConditions(from:) + 5 [inlined]
6   swift-build                   	0x0000000107141f72 PackageBuilder.buildSettings(for:targetRoot:) + 242 (PackageBuilder.swift:820)
7   swift-build                   	0x00000001071410b6 PackageBuilder.createTarget(potentialModule:manifestTarget:dependencies:) + 2438 (PackageBuilder.swift:684)
8   swift-build                   	0x000000010713f25f PackageBuilder.createModules(_:) + 6111 (PackageBuilder.swift:618)
9   swift-build                   	0x000000010713cfaa PackageBuilder.constructV4Targets() + 1098 (PackageBuilder.swift:531)
10  swift-build                   	0x0000000107139065 PackageBuilder.constructTargets() + 1653 (PackageBuilder.swift:438)
11  swift-build                   	0x00000001071388b4 PackageBuilder.construct() + 20 (PackageBuilder.swift:289)
12  swift-build                   	0x00000001070c055d closure #1 in closure #5 in PackageGraphLoader.load(root:config:additionalFileRules:externalManifests:requiredDependencies:unsafeAllowedPackages:remoteArtifacts:xcTestMinimumDeploymentTargets:diagnostics:fileSystem:shouldCreateMultipleTestProducts:createREPLProduct:) + 29 (PackageGraphLoader.swift:186)

Perhaps we can add these conditionally using #if os(windows) as a workaround?

@neonichu
Copy link
Contributor

I agree with Ankit here, we have a fix in master, but it's too late for 5.3 at this point and this would break contributors for a while, especially in Xcode where they don't have the remedy of building their own copy of SwiftPM or downloading a toolchain.

@compnerd compnerd deleted the llvm-on-win32 branch May 23, 2025 00:25
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.

6 participants