You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/02/23 08:13:16 UTC
[GitHub] [arrow] mbrobbel opened a new pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
mbrobbel opened a new pull request #12494:
URL: https://github.com/apache/arrow/pull/12494
As suggested in https://github.com/apache/arrow/pull/12322#issuecomment-1048527114.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mbrobbel commented on pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
Posted by GitBox <gi...@apache.org>.
mbrobbel commented on pull request #12494:
URL: https://github.com/apache/arrow/pull/12494#issuecomment-1048622127
@jvanstraten
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #12494:
URL: https://github.com/apache/arrow/pull/12494#issuecomment-1048600610
@kszucs it seems that (most?) other uses of URL in our cmake files also includes a URL_HASH. Should we add that for substrait as well?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on a change in pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
Posted by GitBox <gi...@apache.org>.
kszucs commented on a change in pull request #12494:
URL: https://github.com/apache/arrow/pull/12494#discussion_r812683871
##########
File path: cpp/src/arrow/engine/CMakeLists.txt
##########
@@ -48,8 +48,7 @@ set(SUBSTRAIT_PROTOS
type_expressions)
externalproject_add(substrait_ep
- GIT_REPOSITORY "${ARROW_SUBSTRAIT_REPO}"
- GIT_TAG "${ARROW_SUBSTRAIT_TAG}"
+ URL "https://github.com/${ARROW_SUBSTRAIT_REPO}/archive/${ARROW_SUBSTRAIT_TAG}.tar.gz"
Review comment:
This url will only work for github so we my want to restore the previous url and note that `ARROW_SUBSTRAIT_REPO` should be a github `user/repo` pair.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mbrobbel closed pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
Posted by GitBox <gi...@apache.org>.
mbrobbel closed pull request #12494:
URL: https://github.com/apache/arrow/pull/12494
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #12494:
URL: https://github.com/apache/arrow/pull/12494#issuecomment-1048619565
I think the purpose of the `ARROW_SUBSTRAIT_REPO` variable is that the developer can quickly switch to a substrait fork with altered protobuf definitions. Introducing a checksum variable would make that less convenient.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12494:
URL: https://github.com/apache/arrow/pull/12494#issuecomment-1048534589
https://issues.apache.org/jira/browse/ARROW-15760
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #12494:
URL: https://github.com/apache/arrow/pull/12494#discussion_r812649474
##########
File path: cpp/src/arrow/engine/CMakeLists.txt
##########
@@ -48,8 +48,7 @@ set(SUBSTRAIT_PROTOS
type_expressions)
externalproject_add(substrait_ep
- GIT_REPOSITORY "${ARROW_SUBSTRAIT_REPO}"
- GIT_TAG "${ARROW_SUBSTRAIT_TAG}"
+ URL "https://github.com/${ARROW_SUBSTRAIT_REPO}/archive/${ARROW_SUBSTRAIT_TAG}.tar.gz"
Review comment:
```suggestion
URL "${ARROW_SUBSTRAIT_REPO}/archive/${ARROW_SUBSTRAIT_TAG}.tar.gz"
```
(the https://github.com/ is included in the variable)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jvanstraten commented on pull request #12494: ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)
Posted by GitBox <gi...@apache.org>.
jvanstraten commented on pull request #12494:
URL: https://github.com/apache/arrow/pull/12494#issuecomment-1048788164
Uh oh, looks like there's been some redundant, parallel work going on... See https://github.com/apache/arrow/pull/12457
tl;dr: the whole setup with the ARROW_SUBSTRAIT_REPO option and such was in and of itself not very conformant to the way the rest of Arrow's build works, and the git shenanigans broke a lot more than just failure to build without git when doing inline builds. Above PR converts the entire build for Substrait to the more usual `ThirdpartyToolchain.cmake` workflow and should supersede this. AFAIK it's ready to merge.
> I think the purpose of the ARROW_SUBSTRAIT_REPO variable is that the developer can quickly switch to a substrait fork with altered protobuf definitions. Introducing a checksum variable would make that less convenient.
It wouldn't work without changing code (including CMake stuff) anyway, since Substrait is still too volatile for that. That said, it'd be useful to have a proper way to override dependency versions and download URL patterns using options for development purposes, which ideally would also just disable the hash check.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org