Skip to content

[cmake] Use custom command for copying files #2398

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

Conversation

drodriguez
Copy link
Contributor

The CMake file(COPY will only copy files during the initial
configuration, so changes in the headers of CF will not be propagated to
the build directory and will make for confussing compilation errors.

Using custom commands should keep the dependencies between files
correctly updated and Ninja should copy the modified headers during each
build if necessary.

@compnerd: There might be something about file(COPY that I was misunderstanding, but I found that the headers weren't updated with my changes, and I needed to do this change for #2397 to work.

The CMake `file(COPY` will only copy files during the initial
configuration, so changes in the headers of CF will not be propagated to
the build directory and will make for confussing compilation errors.

Using custom commands should keep the dependencies between files
correctly updated and Ninja should copy the modified headers during each
build if necessary.
@drodriguez drodriguez requested a review from compnerd July 9, 2019 00:27
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Sounds good

@drodriguez
Copy link
Contributor Author

@swift-ci please test Linux platform

@drodriguez drodriguez merged commit 701699c into swiftlang:master Jul 12, 2019
@drodriguez drodriguez deleted the cmake-fix-header-dependencies branch July 12, 2019 17:23
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