ORC-1618: Disable building tests for snappy #1792
Conversation
| @@ -99,7 +99,7 @@ else () | |||
|
|
|||
| ExternalProject_Add (snappy_ep | |||
| URL "https://github.com/google/snappy/archive/${SNAPPY_VERSION}.tar.gz" | |||
| CMAKE_ARGS ${SNAPPY_CMAKE_ARGS} | |||
| CMAKE_ARGS ${SNAPPY_CMAKE_ARGS} -DSNAPPY_BUILD_TESTS=OFF | |||
There was a problem hiding this comment.
Thank you for suggestion, but may I ask why we need to upgrade to 1.1.8 when we disable like this?
There was a problem hiding this comment.
I did it because it seems not to have negative impact, but I don't think it's needed to fix the issue. I can change the PR not to include this version bump if you prefer.
TBF, I started by trying to update this dep to see if it would fix the actual issue. It did not (and even worse, versions > 1.1.8 do not work at all with the actual build system) but I've let this minor update because it was the state of the repo when I made my tests and so.
There was a problem hiding this comment.
Thanks for the explanation! Do you mind creating a JIRA for this?
There was a problem hiding this comment.
Sorry, I don't have a Jira account (and I'd rather not have to create one just for this very simple MR).
There was a problem hiding this comment.
Understood. Let me do that for you, @douardda .
Disabling the building of tests is a workaround for issue apache#1791 and is probably not an issue since snappy tests are not executed as part of the build process of orc c++ libs.
|
I've updated the PR to only keep the 'disable build tests' part. |
Thank you for update, @douardda . Please update the PR title and description accordingly. |
douardda
changed the title
dongjoon-hyun
changed the title
Disabling the building of tests is a workaround for issue #1791 and is probably not an issue since snappy tests are not executed as part of the build process of orc c++ libs. Note that snappy 1.1.9 and 1.1.10 cannot be used as is because source assets on snappy's gh page are not buildable out of the box, so stick to 1.1.8 for now. ### What changes were proposed in this pull request? Disable building snappy tests as a workaround for issue #1791. ### Why are the changes needed? compilation broken in some situations otherwise (see issue) ### How was this patch tested? compile & unit tests ### Was this patch authored or co-authored using generative AI tooling? no Closes #1792 from douardda/fix-snappy. Authored-by: David Douard <david.douard@sdfa3.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 9627a24) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Disabling the building of tests is a workaround for issue #1791 and is probably not an issue since snappy tests are not executed as part of the build process of orc c++ libs. Note that snappy 1.1.9 and 1.1.10 cannot be used as is because source assets on snappy's gh page are not buildable out of the box, so stick to 1.1.8 for now. ### What changes were proposed in this pull request? Disable building snappy tests as a workaround for issue #1791. ### Why are the changes needed? compilation broken in some situations otherwise (see issue) ### How was this patch tested? compile & unit tests ### Was this patch authored or co-authored using generative AI tooling? no Closes #1792 from douardda/fix-snappy. Authored-by: David Douard <david.douard@sdfa3.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 9627a24) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
|
Merged to main/2.0/1.9. |
Disabling the building of tests is a workaround for issue #1791 and is probably not an issue since snappy tests are not executed as part of the build process of orc c++ libs.
Note that snappy 1.1.9 and 1.1.10 cannot be used as is because source assets on snappy's gh page are not buildable out of the box, so stick to 1.1.8 for now.
What changes were proposed in this pull request?
Disable building snappy tests as a workaround for issue #1791.
Why are the changes needed?
compilation broken in some situations otherwise (see issue)
How was this patch tested?
compile & unit tests
Was this patch authored or co-authored using generative AI tooling?
no