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

rbenv scripts no longer allowed in another directory besides <bin_dir>/../libexec #1437

Closed
konsolebox opened this issue Sep 26, 2022 · 10 comments · Fixed by #1439
Closed

rbenv scripts no longer allowed in another directory besides <bin_dir>/../libexec #1437

konsolebox opened this issue Sep 26, 2022 · 10 comments · Fixed by #1439

Comments

@konsolebox
Copy link

Specifically in this change: 117a381#diff-91b53c00df854a3fb9a4dc150a0a3e37d6c6df79aa36b3731f492b7e23a7ffed

-bin_path="$(abs_dirname "$0")"
+rbenv_bin="${BASH_SOURCE:-$0}"
+bin_path="$(cd "${rbenv_bin%/*}"/../libexec && pwd -P)"

It no longer resolves the rbenv symbolic link and gets the path to the real rbenv script before getting the directory of it, and hardcodingly forces the libexec scripts to be only placed in [rbenv_bin_dir]/../libexec.

Because of this, if the rbenv symbolic link is installed by a distro's package manager or script to /usr/bin/rbenv, the libexec scripts can also only be installed to /usr/libexec, and not in custom locations like /usr/lib/rbenv/libexec. Debian as an example uses this location. See https://packages.debian.org/sid/all/rbenv/filelist.

I also have an ebuild for Gentoo where I place the libexec scripts to /usr/libexec/rbenv instead of /usr/libexec.

I suggested a solution in the commit to have bin/rbenv installed as a script instead of a symbolic link and then make it source the real rbenv in the preferred location configured by the package. This will allow the location of the real script to be known through BASH_SOURCE.

bin/rbenv:

#!/usr/bin/env bash
source ../libexec/rbenv # Can be modified by package manager

libexec/rbenv:

bin_path=$(cd "${BASH_SOURCE%/*}" && pwd -P)
@konsolebox konsolebox changed the title https://github.com/rbenv/rbenv/pull/1428 no longer allows rbenv scripts to be placed on another directory besides <bin_dir>/../libexec #1428 no longer allows rbenv scripts to be placed on another directory besides <bin_dir>/../libexec Sep 26, 2022
@mislav
Copy link
Member

mislav commented Sep 26, 2022

Thanks for reporting!

I better understand the breakage now and I really wish I could resolve BASH_SOURCE to a canonical path in case bin/rbenv is a symlink from another location. I could restore the previous behavior, which used readlink/greadlink to resolve symlinks, but that adds an overhead of an external call to every rbenv invocation.

Alternatively, do you think I could somehow communicate to package maintainers that a bin/rbenv symlink to /path/to/rbenv/libexec/rbenv isn't supported anymore and that they should manually make a bin/rbenv script that just executes /path/to/rbenv/libexec/rbenv?

#!/usr/bin/env sh
# GENERATED BY PACKAGE
exec /path/to/rbenv/libexec/rbenv "$@"

It's a solution that's immediately available to them and I wouldn't have to change anything in this codebase to support it.

@konsolebox
Copy link
Author

So do you plan to not change anything at all and force /path/to/libexec/rbenv to be called by a custom script? Wouldn't that still fail because of current code calculating bin_path with $BASH_SOURCE and ../libexec?

It wouldn't work with /usr/libexec/rbenv/rbenv at least.

@mislav
Copy link
Member

mislav commented Sep 26, 2022

@konsolebox Calling /path/to/rbenv/libexec/rbenv will always work, no matter where rbenv was installed to.

bin_path will get resolved to /path/to/rbenv/libexec, then pushed to PATH during rbenv execution time, making all rbenv-* subcommands available for invocation.

Is there something that I am missing?

@konsolebox
Copy link
Author

To be fair the code in libexec/rbenv can also simply be directly modified and the symbolic link can be kept. I think it's just a bit more formal if it's the calling script that is modified.

As for the package maintainers, I'm not sure if I can say anything about them. I made my own ebuild but it's not a ::gentoo. It's only for my own overlay. Gentoo itself doesn't provide rbenv packages yet. I also just used Debian's rbenv package as a reference when I was still making the ebuild.

@konsolebox
Copy link
Author

@konsolebox Calling /path/to/rbenv/libexec/rbenv will always work, no matter where rbenv was installed to.

bin_path will get resolved to /path/to/rbenv/libexec, then pushed to PATH during rbenv execution time, making all rbenv-* subcommands available for invocation.

Is there something that I am missing?

It will work on that form but if rbenv's path is /usr/libexec/rbenv/rbenv, bin_path will supposedly resolve to /usr/libexec/libexec although it won't because the dir doesn't exist and there will be an error.

@mislav
Copy link
Member

mislav commented Sep 26, 2022

Ah, I understand now what you mean. I don't think I want to support the case where a package maintainer significantly changes the project layout of rbenv codebase. But please let me think about this for a bit!

@mislav mislav changed the title #1428 no longer allows rbenv scripts to be placed on another directory besides <bin_dir>/../libexec rbenv scripts no longer allowed in another directory besides <bin_dir>/../libexec Sep 26, 2022
@hlascelles
Copy link

hlascelles commented Sep 27, 2022

For anyone needing a Googlewhack to find this issue, the exact error line we are seeing (in an Ubuntu Focal container) is:

/usr/local/bin/rbenv: line 45: cd: /usr/local/bin/../libexec: No such file or directory

@mislav
Copy link
Member

mislav commented Sep 27, 2022

@hlascelles Thanks for the info. I wasn't aware that people package rbenv from the latest master branch this way, so I thought I have some time to fix this before cutting a tagged release. But if you experience the master branch to be broken, I can expedite merging #1439

@hlascelles
Copy link

That's OK, no rush! We're just bleeding edge. 🙂

Well use a tag, no problem.

@mislav
Copy link
Member

mislav commented Sep 29, 2022

@hlascelles Resolving symlinks is back in master branch; you may continue to use bleeding edge and please report any problems. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants