[ https://issues.apache.org/jira/browse/IMPALA-11419?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17565115#comment-17565115 ]
Joe McDonnell edited comment on IMPALA-11419 at 7/11/22 7:02 PM: ----------------------------------------------------------------- A problem is that some thrift files generate *_constant.cpp/h files and some don't. One approach is to rearrange this so there is a function that processes a single Thrift file. That function could take in arguments that switch behavior (e.g. an argument that says whether we expect a *constant.cpp/h file or not, arguments that let us turn off some languages, maybe some way to customize thrift args, etc). You end up with something like this: {noformat} ADD_THRIFT_COMPILATION(SomeFile.thrift GENERATES_CONSTANT_FILES true SKIP_JAVA true ) ADD_THRIFT_COMPILATION(SomeOtherFile.thrift) ... more ADD_THRIFT_COMPILATION calls ...{noformat} So, if the function knows that X.thrift doesn't generate constant files, then it can avoid those deps. For the files we patch, we should generate them in a temporary location unpatched, then the patch process should put them in the final location. That way we don't need to think about files being overwritten. I think if we are using add_custom_command and specify OUTPUT_FILE and dependencies appropriately, we can get the incremental part working. In general, Thrift files that need custom actions would replace the ADD_THRIFT_COMPILATION() call with custom code. Edit: Nevermind, fix is already in progress Ideally, the code in common/thrift would populate a variable with the list of files needed for be/generated-sources/gen-cpp/CMakeLists.txt. was (Author: joemcdonnell): A problem is that some thrift files generate *_constant.cpp/h files and some don't. One approach is to rearrange this so there is a function that processes a single Thrift file. That function could take in arguments that switch behavior (e.g. an argument that says whether we expect a *constant.cpp/h file or not, arguments that let us turn off some languages, maybe some way to customize thrift args, etc). You end up with something like this: {noformat} ADD_THRIFT_COMPILATION(SomeFile.thrift GENERATES_CONSTANT_FILES true SKIP_JAVA true ) ADD_THRIFT_COMPILATION(SomeOtherFile.thrift) ... more ADD_THRIFT_COMPILATION calls ...{noformat} So, if the function knows that X.thrift doesn't generate constant files, then it can avoid those deps. For the files we patch, we should generate them in a temporary location unpatched, then the patch process should put them in the final location. That way we don't need to think about files being overwritten. I think if we are using add_custom_command and specify OUTPUT_FILE and dependencies appropriately, we can get the incremental part working. In general, Thrift files that need custom actions would replace the ADD_THRIFT_COMPILATION() call with custom code. Ideally, the code in common/thrift would populate a variable with the list of files needed for be/generated-sources/gen-cpp/CMakeLists.txt. > Incremental build is broken > --------------------------- > > Key: IMPALA-11419 > URL: https://issues.apache.org/jira/browse/IMPALA-11419 > Project: IMPALA > Issue Type: Bug > Components: Infrastructure > Reporter: Zoltán Borók-Nagy > Priority: Major > > IMPALA-11384 broke incremental builds because: > * added a custom target that always rewrites a few generated header files > ** The files are getting included directly/indirectly in most files, so we > always need to recompile a large part of the project > * Didn't remove ${THRIFT_FILE_WE}_constants.cpp/h dependency from > common/thrift/CMakeLists.txt > ** These files are not generated anymore, so the build system always > reconstruct all the generated files (because *_constant.cpp/h is always > missing), and then builds every target that depend on them. > IMPALA-11415 fixes a sporadic error during data loading, but it only covers > the root cause, i.e. we unnecessarily regenerate thrift files. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: issues-all-unsubscr...@impala.apache.org For additional commands, e-mail: issues-all-h...@impala.apache.org