Add redis symlinks at the same place as the installed binaries by vitahlin · Pull Request #193 · valkey-io/valkey

vitahlin · 2024-04-04T02:42:20Z

Adds a new make variable called USE_REDIS_SYMLINKS, with default value yes. If yes, then make install creates additional symlinks to the installed binaries:

valkey-server
valkey-cli
valkey-benchmark
valkey-check-rdb
valkey-check-aof
valkey-sentinel

The names of the symlinks are the legacy redis binary names (redis-server, etc.). The purpose is to provide backward compatibility for scripts expecting the these filenames. The symlinks are installed in the same directory as the binaries (typically /usr/local/bin/ or similar).

Similarly, make uninstall removes these symlinks if USE_REDIS_SYMLINKS is yes.

This is described in a note in README.md.

Fixes #147

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

madolson

I think the approach makes sense, just some minor comments

Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com> Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Signed-off-by: Vitah Lin <vitahlin@gmail.com> Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>

Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com> Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

vitahlin · 2024-04-04T06:42:33Z

I think the approach makes sense, just some minor comments

Solved the language expression problem you suggested and the space problem in Makefile.
Thanks for your help.

zuiderkwast

Yeah, this is the behaviour we want. Some comments on details.

zuiderkwast · 2024-04-04T10:34:23Z

One more thing: The symlinks valkey-check-rdb -> valkey-server and valkey-check-aof -> valkey-server work by conditional code in server.c looking for the exec name, i.e. which symlink was used for starting it. Please also check that it works for redis-check-rdb etc.

Code in server.c:

    /* Check if we need to start in redis-check-rdb/aof mode. We just execute
     * the program main. However the program is part of the Redis executable
     * so that we can easily execute an RDB check on loading errors. */
    if (strstr(exec_name,"valkey-check-rdb") != NULL)
        redis_check_rdb_main(argc,argv,NULL);
    else if (strstr(exec_name,"valkey-check-aof") != NULL)
        redis_check_aof_main(argc,argv);

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

…t to uninstall redis symlink Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

zuiderkwast

Disclaimer: The suggestions are untested. :)

…links Signed-off-by: Vitah Lin <vitahlin@gmail.com>

vitahlin · 2024-04-05T14:31:31Z

One more thing: The symlinks valkey-check-rdb -> valkey-server and valkey-check-aof -> valkey-server work by conditional code in server.c looking for the exec name, i.e. which symlink was used for starting it. Please also check that it works for redis-check-rdb etc.

Code in server.c:
    /* Check if we need to start in redis-check-rdb/aof mode. We just execute
     * the program main. However the program is part of the Redis executable
     * so that we can easily execute an RDB check on loading errors. */
    if (strstr(exec_name,"valkey-check-rdb") != NULL)
        redis_check_rdb_main(argc,argv,NULL);
    else if (strstr(exec_name,"valkey-check-aof") != NULL)
        redis_check_aof_main(argc,argv);

I will test it next.

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

zuiderkwast

Good. Just a few minor suggestions.

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

vitahlin · 2024-04-06T14:28:52Z

My first PR that wasn't about typo. Thank you for your patience and help, have a nice day. 😀

zuiderkwast · 2024-04-06T16:43:38Z

Thanks @vitahlin! I edited the top comment to match what was finally implemented.

…y-io#193) Adds a new make variable called `USE_REDIS_SYMLINKS`, with default value `yes`. If yes, then `make install` creates additional symlinks to the installed binaries: * `valkey-server` * `valkey-cli` * `valkey-benchmark` * `valkey-check-rdb` * `valkey-check-aof` * `valkey-sentinel` The names of the symlinks are the legacy redis binary names (`redis-server`, etc.). The purpose is to provide backward compatibility for scripts expecting the these filenames. The symlinks are installed in the same directory as the binaries (typically `/usr/local/bin/` or similar). Similarly, `make uninstall` removes these symlinks if `USE_REDIS_SYMLINKS` is `yes`. This is described in a note in README.md. Fixes valkey-io#147 --------- Signed-off-by: Vitah Lin <vitahlin@gmail.com> Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>

furai · 2024-04-25T14:24:07Z

For me this change causes issues when using TLS. When I switch off symlinks for redis then installation fails cause it can't find libhiredis_ssl.a. Could this implementation be revisited?

zuiderkwast · 2024-04-25T14:35:31Z

@furai Yes, of course we can revise it. Why is libhiredis_ssl.a affected by this?

furai · 2024-04-25T14:47:14Z

I haven't had time to dig into it. What I'm doing is pretty simple, e.g.:

 make BUILD_TLS=yes USE_SYSTEMD=yes 
 make PREFIX=custom/usr USE_REDIS_SYMLINKS=no install

And then it fails with error because it can't find the aforementioned file. I was building from prepackaged version 7.2.5 available here on github.

vitahlin added 2 commits April 4, 2024 10:06

Chore add support for creating redis symlinks when make install

9cd082c

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Doc add a note about redis synlinks in README.md

a79f162

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

madolson reviewed Apr 4, 2024

View reviewed changes

Comment thread src/Makefile Outdated

Comment thread src/Makefile Outdated

Comment thread README.md Outdated

Comment thread src/Makefile Outdated

vitahlin force-pushed the vitah-chore branch 2 times, most recently from ba81d4d to c4982d3 Compare April 4, 2024 06:29

vitahlin and others added 5 commits April 4, 2024 14:31

Fix rename ENABLE_REDIS_SYMBLINK to CONFIGURE_REDIS_SYMLINKS

feb66fa

Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com> Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Doc update the note of Redis symlink in README.md

c3163ff

Signed-off-by: Vitah Lin <vitahlin@gmail.com> Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>

Style Update src/Makefile style

8d54473

Signed-off-by: Vitah Lin <vitahlin@gmail.com> Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>

Fix Makefile syntax

0cf3fd9

Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com> Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Fix Makefile syntax

7ff0f07

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

vitahlin force-pushed the vitah-chore branch from c4982d3 to 7ff0f07 Compare April 4, 2024 06:34

zuiderkwast reviewed Apr 4, 2024

View reviewed changes

Comment thread README.md Outdated

Comment thread README.md Outdated

Comment thread src/Makefile Outdated

Comment thread src/Makefile Outdated

Comment thread src/Makefile Outdated

zuiderkwast reviewed Apr 4, 2024

View reviewed changes

Comment thread src/Makefile Outdated

vitahlin added 3 commits April 4, 2024 22:17

Doc add md code style and content polish for redis symlink content

87a8688

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Doc remove the word the

415ee24

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Style display symlinks use -> instead of from to

8ebb3c8

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

zuiderkwast linked an issue Apr 4, 2024 that may be closed by this pull request

Create "redis" symlinks for the binaries in make install #147

Closed

vitahlin added 3 commits April 5, 2024 20:30

Refactor define MAYBE_INSTALL_REDIS_SYMLINK to create symlinks or not

5ec6006

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Refactor define MAYBE_UNINSTALL_REDIS_SYMLINK to handle whether or no…

5cf2e77

…t to uninstall redis symlink Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Doc rename CONFIGURE_REDIS_SYMLINKS to USE_REDIS_SYMLINKS

1d66b58

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

zuiderkwast requested changes Apr 5, 2024

View reviewed changes

Comment thread src/Makefile Outdated

Comment thread src/Makefile Outdated

Comment thread src/Makefile Outdated

Comment thread src/Makefile Outdated

Comment thread src/Makefile Outdated

Refactor use ifeq instead of shell if in Makefile to handle redis sym…

9b35629

…links Signed-off-by: Vitah Lin <vitahlin@gmail.com>

zuiderkwast reviewed Apr 5, 2024

View reviewed changes

Comment thread src/Makefile Outdated

vitahlin added 2 commits April 6, 2024 11:49

Refactor use ifeq-yes to determine the value of USE_REDIS_SYMLINKS

f82df63

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

Fix getting redis-check-aof redis-check-rdb symlinks to work correctly

d9ea0b3

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

zuiderkwast reviewed Apr 6, 2024

View reviewed changes

Comment thread README.md Outdated

Comment thread src/server.c

Doc introduce USE_REDIS_SYMLINKS is the makefile variable

80d2f27

Signed-off-by: Vitah Lin <vitahlin@gmail.com>

zuiderkwast approved these changes Apr 6, 2024

View reviewed changes

zuiderkwast added the release-notes This issue should get a line item in the release notes label Apr 6, 2024

zuiderkwast merged commit ba0c93c into valkey-io:unstable Apr 6, 2024

vitahlin deleted the vitah-chore branch April 8, 2024 03:53

zuiderkwast mentioned this pull request Apr 25, 2024

[BUG] Install fails with BUILD_TLS=yes, USE_REDIS_SYMLINKS=no #377

Closed

Uh oh!

Conversation

vitahlin commented Apr 4, 2024 • edited by zuiderkwast Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

madolson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vitahlin commented Apr 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zuiderkwast commented Apr 4, 2024

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vitahlin commented Apr 5, 2024

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vitahlin commented Apr 6, 2024

Uh oh!

zuiderkwast commented Apr 6, 2024

Uh oh!

furai commented Apr 25, 2024

Uh oh!

zuiderkwast commented Apr 25, 2024

Uh oh!

furai commented Apr 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vitahlin commented Apr 4, 2024 •

edited by zuiderkwast

Loading

vitahlin commented Apr 4, 2024 •

edited

Loading