ARROW-15760: [C++] Avoid hard dependency on git in cmake (download tarballs from github instead)

[mbrobbel]

As suggested in #12322 (comment).

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15760

[jorisvandenbossche]

@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?

[kszucs]

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.

[mbrobbel]

@jvanstraten

[jvanstraten]

Uh oh, looks like there's been some redundant, parallel work going on... See #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.

[mbrobbel]

As suggested in #12322 (comment).

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15760

[jorisvandenbossche]

@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?

[kszucs]

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.

[mbrobbel]

@jvanstraten

[jvanstraten]

Uh oh, looks like there's been some redundant, parallel work going on... See #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.

[mbrobbel]

As suggested in #12322 (comment).

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15760

[jorisvandenbossche]

@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?

[kszucs]

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.

[mbrobbel]

@jvanstraten

[jvanstraten]

Uh oh, looks like there's been some redundant, parallel work going on... See #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.

[mbrobbel]

As suggested in #12322 (comment).

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15760

[jorisvandenbossche]

@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?

[kszucs]

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.

[mbrobbel]

@jvanstraten

[jvanstraten]

Uh oh, looks like there's been some redundant, parallel work going on... See #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.

[mbrobbel]

As suggested in #12322 (comment).

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15760

[jorisvandenbossche]

@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?

[kszucs]

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.

[mbrobbel]

@jvanstraten

[jvanstraten]

Uh oh, looks like there's been some redundant, parallel work going on... See #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.

[mbrobbel]

As suggested in #12322 (comment).

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15760

[jorisvandenbossche]

@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?

[kszucs]

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.

[mbrobbel]

@jvanstraten

[jvanstraten]

Uh oh, looks like there's been some redundant, parallel work going on... See #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.

[mbrobbel]

As suggested in #12322 (comment).

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15760

[jorisvandenbossche]

@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?

[kszucs]

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.

[mbrobbel]

@jvanstraten

[jvanstraten]

Uh oh, looks like there's been some redundant, parallel work going on... See #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.

[mbrobbel]

As suggested in #12322 (comment).

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15760

[jorisvandenbossche]

@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?

[kszucs]

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.

[mbrobbel]

@jvanstraten

[jvanstraten]

Uh oh, looks like there's been some redundant, parallel work going on... See #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.