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.

Sign up for GitHub

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

Jump to bottom

Undefined constant COMPRESSION_ZSTD_MIN #2487

Closed
2 tasks done
calebdw opened this issue May 8, 2024 · 3 comments · Fixed by #2490
Closed
2 tasks done

Undefined constant COMPRESSION_ZSTD_MIN #2487

calebdw opened this issue May 8, 2024 · 3 comments · Fixed by #2490
Assignees
@michael-grunder

Comments

@calebdw
Copy link

calebdw commented May 8, 2024

Hello!

It seems that between 5.7.3 and 6.0.0 the constant COMPRESSION_ZSTD_MIN was removed in this commit:

2a6dee5#diff-a1471159ac00556a439382b010c89a8507ebc6f03311ec7655cebd95dc6c978eL336

Was this intentional or just an oversight as COMPRESSION_ZSTD_MAX and COMPRESSION_ZSTD_DEFAULT still remain?

The Laravel framework uses this constant and running the tests locally fails because the constant is no longer defined.

I'm seeing this behaviour on

  • OS: PopOS 22
  • Redis:
  • PHP: 8.2
  • phpredis: 6.0.2

I've checked

  • There is no similar issue from other users
  • Issue isn't fixed in develop branch
@calebdw calebdw mentioned this issue May 8, 2024
Merged
@vlakoff
Copy link

vlakoff commented May 12, 2024

I had look at it, and my guess is that this breakage is an oversight.

@calebdw
Copy link
Author

calebdw commented May 12, 2024

It doesn't look like there's any logic to it, and it seemed to always be set to 1, so not sure how useful it was but just wanted to point it out

@michael-grunder michael-grunder self-assigned this May 13, 2024
@michael-grunder
Copy link
Member

Thanks, I will add this back.

michael-grunder added a commit that referenced this issue May 14, 2024
@michael-grunder
Rework how we declare ZSTD min/max constants.
afa683a 
These values are actually runtime in nature and can be retrieved with
`ZSTD_minCLevel()` and `ZSTD_maxCLevel()` respectively.

Fixes #2487
@michael-grunder michael-grunder mentioned this issue May 14, 2024
Merged
michael-grunder added a commit that referenced this issue May 14, 2024
@michael-grunder
Rework how we declare ZSTD min/max constants.
7e302f8 
These values are actually runtime in nature and can be retrieved with
`ZSTD_minCLevel()` and `ZSTD_maxCLevel()` respectively.

Fixes #2487
vlakoff referenced this issue May 14, 2024
@michael-grunder
Use PHP's new class constant mechanism.
2a6dee5 
Let gen_stub.php define the constants for us, including deriving their
actual values from C defines.

As a side-effect we have to drop support for PHP < 7.2 as it does not
have interned strings.
michael-grunder added a commit that referenced this issue May 16, 2024
@michael-grunder
Rework how we declare ZSTD min/max constants.
2fdedc8 
Fixes #2487
michael-grunder added a commit that referenced this issue May 16, 2024
@michael-grunder
Rework how we declare ZSTD min/max constants.
34b5bd8 
Fixes #2487
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michael-grunder michael-grunder

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rework how we declare ZSTD min/max constants. phpredis/phpredis
3 participants
@michael-grunder @vlakoff @calebdw