You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/12/16 23:58:02 UTC
[GitHub] [tvm] driazati opened a new pull request #9763: Generate compile_commands.json by default
driazati opened a new pull request #9763:
URL: https://github.com/apache/tvm/pull/9763
This is low overhead (it only affects the make-generation step, not the actual build) and makes developer tooling like clangd work much better, so we should have it on by default instead of off (which is CMake's default). If necessary, it can be disabled with
```bash
cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=0 ...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] driazati commented on pull request #9763: Generate compile_commands.json by default
Posted by GitBox <gi...@apache.org>.
driazati commented on pull request #9763:
URL: https://github.com/apache/tvm/pull/9763#issuecomment-996899034
I think it's better in `CMakeLists.txt` since everyone will get it automatically rather than having to update any cmake configs they use, but the point about avoiding messing with parent projects is good. It [looks like](https://stackoverflow.com/questions/25199677/how-to-detect-if-current-scope-has-a-parent-in-cmake) that can be detected so it can avoid setting anything in that case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] driazati commented on a change in pull request #9763: Generate compile_commands.json by default
Posted by GitBox <gi...@apache.org>.
driazati commented on a change in pull request #9763:
URL: https://github.com/apache/tvm/pull/9763#discussion_r777799841
##########
File path: CMakeLists.txt
##########
@@ -110,6 +110,16 @@ include_directories(SYSTEM ${PICOJSON_PATH})
set(TVM_LINKER_LIBS "")
set(TVM_RUNTIME_LINKER_LIBS "")
+
+# Check if this is being run on its own or as a subdirectory for another project
+# If we update to CMake 2.21+, we can use PROJECT_IS_TOP_LEVEL instead
+get_directory_property(IS_SUBPROJECT PARENT_DIRECTORY)
+
+if(NOT IS_SUBPROJECT AND "${CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "")
Review comment:
I don't think it makes any functional difference in this cases but the intent is clearer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #9763: Generate compile_commands.json by default
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #9763:
URL: https://github.com/apache/tvm/pull/9763#issuecomment-996976992
Thank you @driazati for the explanation!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #9763: Generate compile_commands.json by default
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #9763:
URL: https://github.com/apache/tvm/pull/9763#issuecomment-996309780
Yeah usually we need `compile_commands.json` by default, so I'm in favor of doing so. Not a cmake expert, just curious, is it better to put this in CMakeList.txt or config.cmake? What if TVM is used as a CMake submodule by other projects?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a change in pull request #9763: Generate compile_commands.json by default
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #9763:
URL: https://github.com/apache/tvm/pull/9763#discussion_r777740500
##########
File path: CMakeLists.txt
##########
@@ -110,6 +110,16 @@ include_directories(SYSTEM ${PICOJSON_PATH})
set(TVM_LINKER_LIBS "")
set(TVM_RUNTIME_LINKER_LIBS "")
+
+# Check if this is being run on its own or as a subdirectory for another project
+# If we update to CMake 2.21+, we can use PROJECT_IS_TOP_LEVEL instead
+get_directory_property(IS_SUBPROJECT PARENT_DIRECTORY)
+
+if(NOT IS_SUBPROJECT AND "${CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "")
Review comment:
not 100% on this but should we use `DEFINED` instead?
```suggestion
if(NOT IS_SUBPROJECT AND NOT DEFINED "${CMAKE_EXPORT_COMPILE_COMMANDS}")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] driazati edited a comment on pull request #9763: Generate compile_commands.json by default
Posted by GitBox <gi...@apache.org>.
driazati edited a comment on pull request #9763:
URL: https://github.com/apache/tvm/pull/9763#issuecomment-996899034
I think it's better in `CMakeLists.txt` since everyone will get it automatically rather than having to update any cmake configs they use, but the point about avoiding messing with parent projects is good. It [looks like](https://stackoverflow.com/questions/25199677/how-to-detect-if-current-scope-has-a-parent-in-cmake) that can be detected so it can avoid setting anything in that case, I will follow up with a fix soon
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch merged pull request #9763: Generate compile_commands.json by default
Posted by GitBox <gi...@apache.org>.
areusch merged pull request #9763:
URL: https://github.com/apache/tvm/pull/9763
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] driazati commented on a change in pull request #9763: Generate compile_commands.json by default
Posted by GitBox <gi...@apache.org>.
driazati commented on a change in pull request #9763:
URL: https://github.com/apache/tvm/pull/9763#discussion_r777799841
##########
File path: CMakeLists.txt
##########
@@ -110,6 +110,16 @@ include_directories(SYSTEM ${PICOJSON_PATH})
set(TVM_LINKER_LIBS "")
set(TVM_RUNTIME_LINKER_LIBS "")
+
+# Check if this is being run on its own or as a subdirectory for another project
+# If we update to CMake 2.21+, we can use PROJECT_IS_TOP_LEVEL instead
+get_directory_property(IS_SUBPROJECT PARENT_DIRECTORY)
+
+if(NOT IS_SUBPROJECT AND "${CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "")
Review comment:
I don't think it makes any functional difference in this case but the intent is clearer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org