Skip to content
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

Schedule an emit-module-separately job even if an input is not compilable #1588

Merged
merged 3 commits into from
May 1, 2024

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Apr 29, 2024

The following invocation should schedule an emit-module-separately job for src.swift and include other.dylib in the link job.

swiftc src.swift other.dylib -emit-library -emit-module

rdar://127238278

…able

The following invocation should schedule an emit-module-separately job
for src.swift and include other.dylib in the link job.

```
swiftc -emit-library src.swift other.dylib
```

rdar://127238278
@xymus xymus requested a review from artemcm April 29, 2024 19:59
@xymus
Copy link
Contributor Author

xymus commented Apr 29, 2024

@swift-ci Please test

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

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

Thank you.


let emitJob = try plannedJobs.findJob(.emitModule)
XCTAssertTrue(emitJob.commandLine.contains(try toPathOption("foo.swift")))
XCTAssertFalse(emitJob.commandLine.contains(try toPathOption("bar.dylib")))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the dylib an input to the emit module job at all? This seems dangerously close to the behavior of the old driver, which would sometimes pass static archives to the frontend which ends in much sadness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can imagine it being bad, but I think this is fine. Did you notice the XCTAssertFalse for bar.dylib?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, no, sorry, misread it as XCTAssertTrue, which concerned me. You're good, carry on.

Fixes the failure in testDependencyScanningPathRemap.
@xymus
Copy link
Contributor Author

xymus commented Apr 30, 2024

@swift-ci Please test

@xymus
Copy link
Contributor Author

xymus commented Apr 30, 2024

@swift-ci Please test

@xymus
Copy link
Contributor Author

xymus commented Apr 30, 2024

@swift-ci Please test Windows

@xymus xymus merged commit 20926ef into swiftlang:main May 1, 2024
3 checks passed
@xymus xymus deleted the ems-dylibs branch May 1, 2024 03:26
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.

3 participants