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

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

You should be able to delete this entire block now.

99

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

${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

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/