You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@tsfile.apache.org by "jt2594838 (via GitHub)" <gi...@apache.org> on 2025/03/10 05:09:08 UTC
[PR] Refactor header packaging & fix zlib missing header [tsfile]
jt2594838 opened a new pull request, #434:
URL: https://github.com/apache/tsfile/pull/434
Reduce the number or copying header targets.
Fix that Zlib will auto rename zconf.h, which causes a missing header.
--
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: notifications-unsubscribe@tsfile.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor header packaging & fix zlib missing header [tsfile]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 merged PR #434:
URL: https://github.com/apache/tsfile/pull/434
--
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: notifications-unsubscribe@tsfile.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor header packaging & fix zlib missing header [tsfile]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 commented on code in PR #434:
URL: https://github.com/apache/tsfile/pull/434#discussion_r1988235498
##########
cpp/third_party/zlib-1.2.13/zconf.h:
##########
Review Comment:
Well, actually it is not. We just removed it some time ago.
--
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: notifications-unsubscribe@tsfile.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor header packaging & fix zlib missing header [tsfile]
Posted by "ColinLeeo (via GitHub)" <gi...@apache.org>.
ColinLeeo commented on code in PR #434:
URL: https://github.com/apache/tsfile/pull/434#discussion_r1986997788
##########
cpp/third_party/zlib-1.2.13/CMakeLists.txt:
##########
@@ -67,7 +67,8 @@ endif()
if(NOT CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_CURRENT_BINARY_DIR)
# If we're doing an out of source build and the user has a zconf.h
# in their source tree...
- if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h)
+ # only rename in macOS because it conflicts with the embedded header
+ if(APPLE AND EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h)
Review Comment:
May be is unnecessary.
##########
cpp/src/parser/CMakeLists.txt:
##########
@@ -22,4 +22,5 @@ file(GLOB_RECURSE PARSER_SRC_LIST "*.cpp")
add_library(parser_obj OBJECT ${PARSER_SRC_LIST})
file(GLOB_RECURSE HEADERS "${CMAKE_CURRENT_SOURCE_DIR}/*.h")
-copy_to_dir(${HEADERS})
\ No newline at end of file
+SET(PARENT "parser_obj")
Review Comment:
Useless code ?
##########
cpp/third_party/zlib-1.2.13/zconf.h:
##########
Review Comment:
This file is generated by zlib, might be unnecessary.
--
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: notifications-unsubscribe@tsfile.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor header packaging & fix zlib missing header [tsfile]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 commented on code in PR #434:
URL: https://github.com/apache/tsfile/pull/434#discussion_r1988240565
##########
cpp/src/parser/CMakeLists.txt:
##########
@@ -22,4 +22,5 @@ file(GLOB_RECURSE PARSER_SRC_LIST "*.cpp")
add_library(parser_obj OBJECT ${PARSER_SRC_LIST})
file(GLOB_RECURSE HEADERS "${CMAKE_CURRENT_SOURCE_DIR}/*.h")
-copy_to_dir(${HEADERS})
\ No newline at end of file
+SET(PARENT "parser_obj")
Review Comment:
Removed.
--
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: notifications-unsubscribe@tsfile.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refactor header packaging & fix zlib missing header [tsfile]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 commented on code in PR #434:
URL: https://github.com/apache/tsfile/pull/434#discussion_r1988240146
##########
cpp/third_party/zlib-1.2.13/CMakeLists.txt:
##########
@@ -67,7 +67,8 @@ endif()
if(NOT CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_CURRENT_BINARY_DIR)
# If we're doing an out of source build and the user has a zconf.h
# in their source tree...
- if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h)
+ # only rename in macOS because it conflicts with the embedded header
+ if(APPLE AND EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/zconf.h)
Review Comment:
I may try to revert it, but if the CI issue reappears, I will add it back.
--
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: notifications-unsubscribe@tsfile.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org