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

Change redis to server in INFO fields redis_{git_sha1,git_dirty,build_id} #1736

Closed
wants to merge 2 commits into from
Closed

Change redis to server in INFO fields redis_{git_sha1,git_dirty,build_id} #1736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d506dc7
Change redis to server in INFO fields redis_git_sha1, redis_git_dirty…
zuiderkwast Feb 14, 2025
07d3ea9
Fix usage of renamed macro
zuiderkwast Feb 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/mkreleasehdr.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ fi
test -f release.h || touch release.h
(cat release.h | grep SHA1 | grep $GIT_SHA1) && \
(cat release.h | grep DIRTY | grep $GIT_DIRTY) && exit 0 # Already up-to-date
echo "#define REDIS_GIT_SHA1 \"$GIT_SHA1\"" > release.h
echo "#define REDIS_GIT_DIRTY \"$GIT_DIRTY\"" >> release.h
echo "#define REDIS_BUILD_ID \"$BUILD_ID\"" >> release.h
echo "#define SERVER_GIT_SHA1 \"$GIT_SHA1\"" > release.h
echo "#define SERVER_GIT_DIRTY \"$GIT_DIRTY\"" >> release.h
echo "#define SERVER_BUILD_ID \"$BUILD_ID\"" >> release.h
echo "#include \"version.h\"" >> release.h
echo "#define REDIS_BUILD_ID_RAW SERVER_NAME VALKEY_VERSION REDIS_BUILD_ID REDIS_GIT_DIRTY REDIS_GIT_SHA1" >> release.h
echo "#define SERVER_BUILD_ID_RAW SERVER_NAME VALKEY_VERSION SERVER_BUILD_ID SERVER_GIT_DIRTY SERVER_GIT_SHA1" >> release.h
touch release.c # Force recompile of release.c
2 changes: 1 addition & 1 deletion src/rdma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1885,7 +1885,7 @@ int ValkeyModule_OnLoad(void *ctx, ValkeyModuleString **argv, int argc) {
UNUSED(argc);

/* Connection modules MUST be part of the same build as valkey. */
if (strcmp(REDIS_BUILD_ID_RAW, serverBuildIdRaw())) {
if (strcmp(SERVER_BUILD_ID_RAW, serverBuildIdRaw())) {
serverLog(LL_NOTICE, "Connection type %s was not built together with the valkey-server used.", CONN_TYPE_RDMA);
return VALKEYMODULE_ERR;
}
Expand Down
8 changes: 4 additions & 4 deletions src/release.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,19 @@
#include "crc64.h"

char *serverGitSHA1(void) {
return REDIS_GIT_SHA1;
return SERVER_GIT_SHA1;
}

char *serverGitDirty(void) {
return REDIS_GIT_DIRTY;
return SERVER_GIT_DIRTY;
}

const char *serverBuildIdRaw(void) {
return REDIS_BUILD_ID_RAW;
return SERVER_BUILD_ID_RAW;
}

uint64_t serverBuildId(void) {
char *buildid = REDIS_BUILD_ID_RAW;
char *buildid = SERVER_BUILD_ID_RAW;

return crc64(0, (unsigned char *)buildid, strlen(buildid));
}
Expand Down
6 changes: 3 additions & 3 deletions src/server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5669,9 +5669,9 @@ sds genValkeyInfoString(dict *section_dict, int all_sections, int everything) {
"server_name:%s\r\n", SERVER_NAME,
"valkey_version:%s\r\n", VALKEY_VERSION,
"valkey_release_stage:%s\r\n", VALKEY_RELEASE_STAGE,
"redis_git_sha1:%s\r\n", serverGitSHA1(),
"redis_git_dirty:%i\r\n", strtol(serverGitDirty(), NULL, 10) > 0,
"redis_build_id:%s\r\n", serverBuildIdString(),
"server_git_sha1:%s\r\n", serverGitSHA1(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some part of me says let's just leave them. One more thing to not list as a breaking change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember we should keep this info message as is, otherwise is a break change. For the other update in this PR, everything is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's a breaking change.

So I think we will not change it in 9.0 then. But I'm wondering: How long are we going to keep everything 'redis'? Forever? Five years? If we keep it forever, then users will always remember that Redis was the original and Valkey is just a fork.

If we want to accept the risk, we are allowed to make breaking changes in a major version. I understand "redis_version" is used by many clients, but the git fields are likely not used by many. If we decide to change it some time, I hope users will understand that we want to phase out 'redis' named fields.

"server_git_dirty:%i\r\n", strtol(serverGitDirty(), NULL, 10) > 0,
"server_build_id:%s\r\n", serverBuildIdString(),
"%s_mode:", (server.extended_redis_compat ? "redis" : "server"),
"%s\r\n", mode,
"os:%s", name.sysname,
Expand Down
2 changes: 1 addition & 1 deletion src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ int ValkeyModule_OnLoad(void *ctx, ValkeyModuleString **argv, int argc) {
UNUSED(argc);

/* Connection modules must be part of the same build as the server. */
if (strcmp(REDIS_BUILD_ID_RAW, serverBuildIdRaw())) {
if (strcmp(SERVER_BUILD_ID_RAW, serverBuildIdRaw())) {
serverLog(LL_NOTICE, "Connection type %s was not built together with the valkey-server used.", CONN_TYPE_TLS);
return VALKEYMODULE_ERR;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/info-command.tcl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ start_server {tags {"info and its relative command"}} {
test "Extract version and sha1 details from info command and print" {
set i [r info]
regexp {redis_version:(.*?)\r\n} $i - version
regexp {redis_git_sha1:(.*?)\r\n} $i - sha1
regexp {server_git_sha1:(.*?)\r\n} $i - sha1
puts "Testing Valkey version $version ($sha1)"
}

Expand Down
Loading