Skip to content

[Merge Modules] Omit merge-modules job when only one input is given. #577

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

Merged
merged 1 commit into from
Apr 1, 2021

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Apr 1, 2021

And said input matches the expected output path.

Otherwise both the .compile and the .mergeModule jobs have an identical output:
.temporary(<module_name>.swiftmodule) which will trigger a fatal error in the MultiJobExecutor because it may cause LLBuild cycles downstream.

@artemcm artemcm requested a review from xymus April 1, 2021 17:31
@nkcsgexi
Copy link
Contributor

nkcsgexi commented Apr 1, 2021

Are we certain that we will use the correct output file name Foo.swiftmodule instead of filename~particial.swiftmodule in the frontend command?

@artemcm
Copy link
Contributor Author

artemcm commented Apr 1, 2021

Are we certain that we will use the correct output file name Foo.swiftmodule instead of filename~particial.swiftmodule in the frontend command?

We ensure with this check:

soleInput.fileHandle == mergeOutput

That this will only omit merge-modules if the final module output matches that from the compile job.
In a plain compile with one source file and with -emit-module, we will have the situation you describe where we have one module Foo~partial.swiftmodule in a temporary directory somewhere and the output of merge-modules is Foo.swiftmodule where the user wants it. In that case, the job will not be skipped.

This is motivated by the -g case where the final module output is in a temporary directory and matches exactly the module output of the single-source-file.

@artemcm artemcm requested a review from nkcsgexi April 1, 2021 18:05
@nkcsgexi
Copy link
Contributor

nkcsgexi commented Apr 1, 2021

ah, that makes sense. Thank you for the explanation! @artemcm

@artemcm
Copy link
Contributor Author

artemcm commented Apr 1, 2021

@swift-ci please test

@artemcm
Copy link
Contributor Author

artemcm commented Apr 1, 2021

@swift-ci please test

And said input matches the expected output path.
@artemcm
Copy link
Contributor Author

artemcm commented Apr 1, 2021

@swift-ci please test

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.

2 participants