Skip to content

Remove the workaround for swift-package-manager-#8111.#872

Merged
grynspan merged 2 commits into
mainfrom
jgrynspan/remove-8111-workaround
Dec 20, 2024
Merged

Remove the workaround for swift-package-manager-#8111.#872
grynspan merged 2 commits into
mainfrom
jgrynspan/remove-8111-workaround

Conversation

@grynspan

@grynspan grynspan commented Dec 19, 2024

Copy link
Copy Markdown
Contributor

Remove the workaround for swiftlang/swift-package-manager#8111. The issue has been resolved and Windows correctly exports symbols from dynamic library targets.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

Remove the workaround for swiftlang/swift-package-manager#8111.
The issue has been resolved and Windows correctly exports symbols from dynamic library
targets.
@grynspan grynspan added windows 🪟 Windows support workaround Workaround for an issue in another component (may need to revert later) build 🧱 Affects the project's build configuration or process labels Dec 19, 2024
@grynspan grynspan added this to the Swift 6.x milestone Dec 19, 2024
@grynspan grynspan self-assigned this Dec 19, 2024
@grynspan

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@grynspan grynspan requested a review from dschaefer2 December 19, 2024 23:06
Comment thread Package.swift Outdated
@grynspan

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@grynspan grynspan merged commit aa96a79 into main Dec 20, 2024
@grynspan grynspan deleted the jgrynspan/remove-8111-workaround branch December 20, 2024 17:32
@xedin

xedin commented Dec 25, 2024

Copy link
Copy Markdown
Contributor

@grynspan @stmontgomery I wonder if this caused Source Compatibility Suite regressions - https://ci.swift.org/job/swift-main-source-compat-suite-debug/989/artifact/swift-source-compat-suite/

@grynspan

Copy link
Copy Markdown
Contributor Author

I suppose it's possible, but looking at the failures, they don't seem related. It looks like the testing module simply isn't being built, whereas our change here (combined with the follow-up 840c822 patch) just makes the library build as a DLL on Windows. There should be absolutely no visible change on non-Windows systems.

@xedin

xedin commented Dec 25, 2024

Copy link
Copy Markdown
Contributor

I see, so it must be something about ScanDependencies or something related to that.

@grynspan

Copy link
Copy Markdown
Contributor Author

I'm not willing to rule this change out entirely since the follow-up commit might have been delayed making it into a toolchain. But I would definitely look at ScanDependencies first if only to rule it out.

@xedin

xedin commented Dec 25, 2024

Copy link
Copy Markdown
Contributor

Sounds good, thank you!

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

Labels

build 🧱 Affects the project's build configuration or process windows 🪟 Windows support workaround Workaround for an issue in another component (may need to revert later)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants