Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++] Minimal build failed in crossbow because of zstd v1.5.4 requires CMake v3.18 #34148

Closed
mapleFU opened this issue Feb 12, 2023 · 6 comments · Fixed by #34190
Closed

[C++] Minimal build failed in crossbow because of zstd v1.5.4 requires CMake v3.18 #34148

mapleFU opened this issue Feb 12, 2023 · 6 comments · Fixed by #34190

Comments

@mapleFU
Copy link
Member

mapleFU commented Feb 12, 2023

Describe the bug, including details regarding any error messages, version, and platform.

After patch #34114 is merged, crossbow CI was broken: https://github.com/ursacomputing/crossbow/actions/runs/4153851721/jobs/7185765610#step:3:1131

This is because zstd v1.5.4 introduce CheckLinkerFlag, which is introduced in facebook/zstd#3392 . And our crossbow CI install cmake 3.16, which didn't support it.

Component(s)

C++, Developer Tools

@mapleFU
Copy link
Member Author

mapleFU commented Feb 12, 2023

@kou

@mapleFU
Copy link
Member Author

mapleFU commented Feb 12, 2023

Also an issue here: facebook/zstd#3500

@kou
Copy link
Member

kou commented Feb 12, 2023

We can require CMake 3.13 or 3.16 since 12.0.0 (see #33800 ) but it doesn't solve this.

Could you add a CMake version check like https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L1539-L1542 ?

@mapleFU
Copy link
Member Author

mapleFU commented Feb 12, 2023

@kou kou added this to the 12.0.0 milestone Feb 15, 2023
kou pushed a commit that referenced this issue Feb 15, 2023
### Rationale for this change

This patch part reverts #34114 . But it didn't revert the logic for url and sha256 change.

### What changes are included in this PR?

zstd version revert back to v1.5.2

### Are these changes tested?

no

### Are there any user-facing changes?

no

* Closes: #34148

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
gringasalpastor pushed a commit to gringasalpastor/arrow that referenced this issue Feb 17, 2023
### Rationale for this change

This patch part reverts apache#34114 . But it didn't revert the logic for url and sha256 change.

### What changes are included in this PR?

zstd version revert back to v1.5.2

### Are these changes tested?

no

### Are there any user-facing changes?

no

* Closes: apache#34148

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
fatemehp pushed a commit to fatemehp/arrow that referenced this issue Feb 24, 2023
### Rationale for this change

This patch part reverts apache#34114 . But it didn't revert the logic for url and sha256 change.

### What changes are included in this PR?

zstd version revert back to v1.5.2

### Are these changes tested?

no

### Are there any user-facing changes?

no

* Closes: apache#34148

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou
Copy link
Member

kou commented Apr 1, 2023

@mapleFU Zstandard 1.5.5 will be released soon: facebook/zstd#3585
It will include facebook/zstd#3510 .
Could you try upgrading Zstandard again?

@mapleFU
Copy link
Member Author

mapleFU commented Apr 2, 2023

facebook/zstd#3585

Sure, I'm subscribing it's release. Once it send a release, I'll send a patch for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment