You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@tsfile.apache.org by "chrisdutz (via GitHub)" <gi...@apache.org> on 2024/06/12 16:01:51 UTC
[PR] Fix/fix the cpp build on windows [tsfile]
chrisdutz opened a new pull request, #109:
URL: https://github.com/apache/tsfile/pull/109
This PR adds the usage of the cmake-maven-plugin back as well as addresses some code-level changes needed in order to compile TsFile CPP on Windows systems.
Manually tested with `mvnw -P with-cpp clean install` on:
- Windows aarch64
- Windows intel
- Linux aarch64
- Linux intel
- MacOS aarch64
- MacOS intel
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #109:
URL: https://github.com/apache/tsfile/pull/109#issuecomment-2164744903
Ok ... Windows GitHub Action builds enabled and working (Had to do a minor addition to one of the functions in order to make it work however)
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #109:
URL: https://github.com/apache/tsfile/pull/109#issuecomment-2164679309
I guess the probably most important part is to either ask the author of the code snippet to flag his repo as Apache licensed, or to send us a copy of it with an apache license header ... right now it's technically under the GitHub default license and that's not quite compatible with the Apache one.
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #109:
URL: https://github.com/apache/tsfile/pull/109#issuecomment-2166815048
Ok... I manged to reach the author of the patch and he just set the license of his repo to apache 2.0, so I guess we're good to merge the pr
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "jt2594838 (via GitHub)" <gi...@apache.org>.
jt2594838 merged PR #109:
URL: https://github.com/apache/tsfile/pull/109
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "ColinLeeo (via GitHub)" <gi...@apache.org>.
ColinLeeo commented on PR #109:
URL: https://github.com/apache/tsfile/pull/109#issuecomment-2164398045
Excellent job!
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on code in PR #109:
URL: https://github.com/apache/tsfile/pull/109#discussion_r1639343480
##########
cpp/src/file/read_file.cc:
##########
@@ -123,4 +130,32 @@ int ReadFile::read(int32_t offset, char *buf, int32_t buf_size,
return ret;
}
-} // end namespace storage
\ No newline at end of file
+} // end namespace storage
+
+// https://github.com/aleitner/windows_pread/blob/master/src/pread.c
+// We need to find out which license this is under.
Review Comment:
Will take care of that
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou commented on PR #109:
URL: https://github.com/apache/tsfile/pull/109#issuecomment-2172548985
> So? We good to merge?
@jt2594838
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou commented on code in PR #109:
URL: https://github.com/apache/tsfile/pull/109#discussion_r1639101296
##########
cpp/src/file/read_file.cc:
##########
@@ -123,4 +130,32 @@ int ReadFile::read(int32_t offset, char *buf, int32_t buf_size,
return ret;
}
-} // end namespace storage
\ No newline at end of file
+} // end namespace storage
+
+// https://github.com/aleitner/windows_pread/blob/master/src/pread.c
+// We need to find out which license this is under.
Review Comment:
ok, so we can also change or remove this comment of L136?
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #109:
URL: https://github.com/apache/tsfile/pull/109#issuecomment-2172465617
So? We good to merge?
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou commented on PR #109:
URL: https://github.com/apache/tsfile/pull/109#issuecomment-2164161673
Thanks Chris! Consider enabling the CI on windows then?
https://github.com/apache/tsfile/blob/e17e373f3e0e6354e8fe39d437086bf78bc521e3/.github/workflows/unit-test.yml#L94-L98
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "HTHou (via GitHub)" <gi...@apache.org>.
HTHou commented on code in PR #109:
URL: https://github.com/apache/tsfile/pull/109#discussion_r1639101296
##########
cpp/src/file/read_file.cc:
##########
@@ -123,4 +130,32 @@ int ReadFile::read(int32_t offset, char *buf, int32_t buf_size,
return ret;
}
-} // end namespace storage
\ No newline at end of file
+} // end namespace storage
+
+// https://github.com/aleitner/windows_pread/blob/master/src/pread.c
+// We need to find out which license this is under.
Review Comment:
ok, so we can also change or remove this comment?
--
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] Fix/fix the cpp build on windows [tsfile]
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on code in PR #109:
URL: https://github.com/apache/tsfile/pull/109#discussion_r1639373155
##########
cpp/src/file/read_file.cc:
##########
@@ -123,4 +130,32 @@ int ReadFile::read(int32_t offset, char *buf, int32_t buf_size,
return ret;
}
-} // end namespace storage
\ No newline at end of file
+} // end namespace storage
+
+// https://github.com/aleitner/windows_pread/blob/master/src/pread.c
+// We need to find out which license this is under.
Review Comment:
Done
--
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