This is an archive of the discontinued LLVM Phabricator instance.

Accept any filepath in llvm_check_source_file_list
ClosedPublic

Authored by serge-sans-paille on Mar 19 2018, 7:38 AM.

Details

Summary

llvm_check_source_file_list currently only accepts paths relative to current CMAKE_SOURCE_DIR. Extend it to any path, including absolute ones.

Diff Detail

Repository
rL LLVM

Event Timeline

Could you upload the patch with context (-U9999) please?

smeenai accepted this revision.Mar 19 2018, 12:09 PM

LGTM with the comments addressed.

cmake/modules/LLVMProcessSources.cmake
84–89 ↗(On Diff #138948)

You should be able to delete this entire block now.

99 ↗(On Diff #138948)

It's more friendly to display the relative path to the source directory here, rather than the full path.

This revision is now accepted and ready to land.Mar 19 2018, 12:09 PM

Error message updated.

cmake/modules/LLVMProcessSources.cmake
84–89 ↗(On Diff #138948)

${entry} is still used in `list(FIND LLVM_OPTIONAL_SOURCES ${entry} idx)`. I don't quite get where LLVM_OPTIONAL_SOURCES is set though.

serge-sans-paille marked an inline comment as done.Mar 19 2018, 1:36 PM
smeenai added inline comments.Mar 19 2018, 1:47 PM
cmake/modules/LLVMProcessSources.cmake
99 ↗(On Diff #138997)

I think ${g} is still gonna be the full path, since it's the result of a glob. You'll need to do an explicit relative path calculation.

Error message checked on our codebase without issue. Indeed a relative path looks prettier.

Sweet, thanks for the update.

This revision was automatically updated to reflect the committed changes.

thanks @smeenai for the review o/