Fix "Add emit extension block symbols option to dump-symbol-graph and SymbolGraphOptions"

[theMomax]

Fixes #5892. Reverts #5975. The Windows build failure had nothing to do with the original PR #5892.

The previous windows build for #5950, which does not include the changes from #5892 fails in exactly the same way.

[theMomax]

@neonichu please run tests to see if the windows error still shows up. You already noted for #5950 that this error seems to be caused somewhere else: #5950 (comment).

[neonichu]

@swift-ci please smoke test

[neonichu]

I think the problem @compnerd was talking about was an issue with finding the SwiftDriver module. The CMake configuration looks reasonable to me (Commands is declaring linkage to DriverSupport which in turn declares linkage of SwiftDriver publicly), but maybe we have to tell CMake again that it is supposed to link SwiftDriver?

[theMomax]

I see, so there were two issues, the first of which is gone now.

The CMake configuration looks reasonable to me (Commands is declaring linkage to DriverSupport which in turn declares linkage of SwiftDriver publicly), but maybe we have to tell CMake again that it is supposed to link SwiftDriver?

I really don't know anything about CMake, but in the CoreCommands CMakeLists SwiftDriver isn't listed explicitly and it works, so I guess that can't be the problem.

I tried adding the @_implementationOnly attribute to the DriverSupport import I added, since DriverSupport is only listed as a private dependency in CMake.

[MaxDesiatov]

@swift-ci smoke test Windows

[MaxDesiatov]

@swift-ci smoke test

[MaxDesiatov]

@swift-ci smoke test Windows platform

[theMomax]

@MaxDesiatov looks like my fix worked and the windows build passed, but it's not reported in the Github CI overview. Is that expected?

[MaxDesiatov]

No, it's not expected, unfortunately.

[MaxDesiatov]

@swift-ci smoke test Windows platform

[compnerd]

It would be nice to run a packaging test I think. The UB in the frontend had prevented toolchain updates for a bit, and SPM regressed a fair amount in that time period. I really would like to get a toolchain update rolled out before the freeze.

[compnerd]

I'm not sure if this change is responsible or not, but between the build on 12/12 and this change, smoke testing with this change I see:

error: Invalid manifest (compiled with: ["C:\\Library\\Developer\\Toolchains\\unknown-Asserts-development.xctoolchain\\usr\\bin\\swiftc.exe", "-vfsoverlay", "C:\\Users\\user\\AppData\\Local\\Temp\\TemporaryDirectory.V0ll6f\\vfs.yaml", "-L", "C:\\Library\\Developer\\Toolchains\\unknown-Asserts-development.xctoolchain\\usr\\lib\\swift\\pm\\ManifestAPI", "-lPackageDescription", "-sdk", "C:\\Library\\Developer\\Platforms\\Windows.platform\\Developer\\SDKs\\Windows.sdk", "-libc", "MD", "-I", "C:\\Library\\Developer\\Platforms\\Windows.platform\\Developer\\Library\\XCTest-development\\usr\\lib\\swift\\windows", "-I", "C:\\Library\\Developer\\Platforms\\Windows.platform\\Developer\\Library\\XCTest-development\\usr\\lib\\swift\\windows\\x86_64", "-L", "C:\\Library\\Developer\\Platforms\\Windows.platform\\Developer\\Library\\XCTest-development\\usr\\lib\\swift\\windows\\x86_64", "-swift-version", "5", "-I", "C:\\Library\\Developer\\Toolchains\\unknown-Asserts-development.xctoolchain\\usr\\lib\\swift\\pm\\ManifestAPI", "-package-description-version", "5.0.0", "\\Package.swift", "-Xfrontend", "-disable-implicit-concurrency-module-import", "-Xfrontend", "-disable-implicit-string-processing-module-import", "-o", "C:\\Users\\user\\AppData\\Local\\Temp\\TemporaryDirectory.zun9HV\\swift-log-manifest.exe"])
\Package.swift:24:18: error: the compiler is unable to type-check this expression in reasonable time; try breaking up the expression into distinct sub-expressions
let SwiftWin32 = Package(
                 ^~~~~~~~ in https://github.com/apple/swift-log.git

[theMomax]

I'm not sure if this change is responsible or not, but between the build on 12/12 and this change, smoke testing with this change I see:

This PR doesn't change anything about the Package manifest definition.

[compnerd]

Looked a bit more into this issue with @neonichu. This ended up being related to the VFS handling. I think that this is really a shady workaround, but it does build 🤷‍♂️. There really should be a tracking dependency in CMake for this as this is brittle and will likely need to be reverted pretty soon on main.

[theMomax]

@compnerd thanks for looking into it. @neonichu can you merge this, please?

[MaxDesiatov]

As a gentle reminder, do you need to cherry-pick this for the 5.8 branch? If so, you'll have to create a new PR against that branch.

[theMomax]

@MaxDesiatov, yes! I guess I just have to open another PR to the release branch?

[MaxDesiatov]

Yes, unless @neonichu and @tomerd have any objections against cherry-picking this for 5.8. To me personally, this looks like a relatively small change that was already there but was reverted and didn't get in because of an unrelated CI breakage, if I understand correctly.

[theMomax]

I opened the PR for Swift 5.8: #5985. @neonichu, @tomerd please take a look. Thanks!