fix: use realpath for building path of bin-file#49
Conversation
if use virtual-store outside of node_modules and node_modules outside of build dir, bin path can be broke for example we have this hypothetical structure ``` ~/Projects/app/node_modules -> ~/.cache/modules/app/node_modules ~/.cache/modules/app/node_modules/eslint -> ~/.cache/vstore/app/eslint@8.56.0/node_modules/eslint ~/.cache/modules/app/node_modules/.bin/eslint => $0 => node_modules/.bin/eslint basedir => node_modules/.bin ... node "node_modules/.bin/../../../../vstore/app/eslint@8.56.0/node_modules/eslint/bin/eslint.js" => error, module not found ``` module not found, because path is incorrect `~/Projects/app/node_modules/.bin/../../../../vstore/app/eslint@8.56.0/node_modules/eslint/bin/eslint.js` with realpath it will fixed ``` realpath $0 => ~/.cache/modules/app/node_modules/.bin/eslint basedir => ~/.cache/modules/app/node_modules/.bin node ~/.cache/modules/app/node_modules/.bin/../../../../vstore/app/eslint@8.56.0/node_modules/eslint/bin/eslint.js => no errors ``` ps: we have node_modules outside of our build dir for some optimisation
src/index.ts
Outdated
| let sh = `\ | ||
| #!/bin/sh | ||
| basedir=$(dirname "$(echo "$0" | sed -e 's,\\\\,/,g')") | ||
| basedir=$(dirname "$(realpath "$0" | sed -e 's,\\\\,/,g')") |
There was a problem hiding this comment.
is the realpath command available on all supported systems?
can this be fixed in https://github.com/pnpm/pnpm/tree/main/pkg-manager/link-bins instead?
There was a problem hiding this comment.
You are right, some system may have no realpath.
May be we can use something like this?
get_path() {
if command -v realpath >/dev/null 2>&1; then
realpath "$1"
elif command -v readlink >/dev/null 2>&1; then
# some systems has readlink, but doesn't have "-f" flag
if readlink -f "$1" >/dev/null 2>&1; then
readlink -f "$1"
else
readlink "$1"
fi
elif command -v perl >/dev/null 2>&1; then
perl -e 'use Cwd "abs_path"; print abs_path(shift)' "$1"
elif [[ $1 = /* ]]; then
echo "$1"
else
echo "$PWD/${1#./}"
fi
}
basedir=$(dirname "$(get_path "$0" | sed -e 's,\\\\,/,g')")or
basedir=$(dirname "$(echo "$0" | sed -e 's,\\\\,/,g')")
case \`uname\` in
*CYGWIN*|*MINGW*|*MSYS*)
if command -v cygpath > /dev/null 2>&1; then
basedir=\`cygpath -w "$basedir"\`
fi
;;
*)
if command -v realpath >/dev/null 2>&1; then
basedir=`realpath "$basedir"`
elif command -v readlink >/dev/null 2>&1; then
# some systems has the readlink, but doesn't have "-f" flag
if readlink -f "$basedir" >/dev/null 2>&1; then
basedir=`readlink -f "$basedir"`
else
basedir=`readlink "$basedir"`
fi
elif command -v perl >/dev/null 2>&1; then
basedir=`perl -e 'use Cwd "abs_path"; print abs_path(shift)' "$basedir"`
elif [[ $basedir != /* ]]; then
basedir="$PWD/${basedir#./}"
fi
;;
esacThere was a problem hiding this comment.
can this be fixed in https://github.com/pnpm/pnpm/tree/main/pkg-manager/link-bins instead?
I think we can't, because it can broke portable resource (build+node_modules).
So we cannot calculate it in the compile time. We can calculate it only in the runtime.
| @@ -1,4 +1,4 @@ | |||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | |||
| // Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing | |||
There was a problem hiding this comment.
it was made by jest --updateSnapshot command
|
You have updated only the sh command shim. What about cmd and powershell? Also, I don't feel I am competent enough in this area, so I'd wait for opinion from other contributors. |
if use virtual-store outside of node_modules and node_modules outside of build dir, bin path can be broke
for example we have this hypothetical structure
module not found, because path is incorrect
~/Projects/app/node_modules/.bin/../../../../vstore/app/eslint@8.56.0/node_modules/eslint/bin/eslint.jswith realpath it will fixed
ps: we have node_modules outside of our build dir for some optimisation