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

Can't build from source with crystal installed from snap #13592

Closed
koffeinfrei opened this issue Jun 21, 2023 · 5 comments · Fixed by #13596
Closed

Can't build from source with crystal installed from snap #13592

koffeinfrei opened this issue Jun 21, 2023 · 5 comments · Fixed by #13596
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:infrastructure

Comments

@koffeinfrei
Copy link
Contributor

koffeinfrei commented Jun 21, 2023

I followed Install from sources with Crystal installed from snap.

$ make
Using /usr/bin/llvm-config-15 [version= 15.0.7]
g++ -c  -o src/llvm/ext/llvm_ext.o src/llvm/ext/llvm_ext.cc -I/usr/lib/llvm-15/include -std=c++14   -fno-exceptions -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
CRYSTAL_CONFIG_BUILD_COMMIT="df960cf76" CRYSTAL_CONFIG_PATH='$ORIGIN/../share/crystal/src' SOURCE_DATE_EPOCH="1687369226" CC="cc -fuse-ld=lld" CRYSTAL_CONFIG_LIBRARY_PATH='$ORIGIN/../lib/crystal' ./bin/crystal build -D strict_multi_assign -D preview_overload_order -Dwithout_interpreter  -o .build/crystal src/compiler/crystal.cr -D without_openssl -D without_zlib -D use_pcre2
error: unknown command "build", see 'snap help'.
make: *** [Makefile:214: .build/crystal] Error 64

The issue is that the realpath command for the following line

PARENT_CRYSTAL_COMMAND=$(realpath "$(command -v "$PARENT_CRYSTAL" 2> /dev/null)" 2> /dev/null)

resolves to /usr/bin/snap.

The following rough patch for the binstub resolves the issue:

diff --git a/bin/crystal b/bin/crystal
index 8e42d4e7e..800c1277c 100755
--- a/bin/crystal
+++ b/bin/crystal
@@ -154,6 +154,9 @@ if [ -z "${PARENT_CRYSTAL##*/*}" -a "$(realpath "$PARENT_CRYSTAL")" = "$SCRIPT_P
 fi
 
 PARENT_CRYSTAL_COMMAND=$(realpath "$(command -v "$PARENT_CRYSTAL" 2> /dev/null)" 2> /dev/null)
+if [ $(basename "$PARENT_CRYSTAL_COMMAND") = "snap" ]; then
+  PARENT_CRYSTAL_COMMAND="$(command -v "$PARENT_CRYSTAL" 2> /dev/null)"
+fi
 PARENT_CRYSTAL_EXISTS=$(test !$?)
 
 # check if the parent crystal command refers to this script

There's probably a more proper way to handle the snap case. If that's good enough I'm happy to submit a PR. Otherwise I'm open to suggestions on how to improve the patch.

@koffeinfrei koffeinfrei added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Jun 21, 2023
@straight-shoota
Copy link
Member

Can you give some insights into why the realpath of PARENT_CRYSTAL ends up being /usr/bin/snap?
I suppose it might have something to do with how snap operates (which I have no idea about).

@koffeinfrei
Copy link
Contributor Author

I'm not familiar with how snap binaries work internally, but it's a sandboxed execution environment and somehow goes through the snap binary.

$ which crystal
/snap/bin/crystal

$ readlink /snap/bin/crystal
/usr/bin/snap

$ ls -l /snap/bin/crystal
lrwxrwxrwx 1 root root 13 Mai  9 14:18 /snap/bin/crystal -> /usr/bin/snap

$ realpath /snap/bin/crystal
/usr/bin/snap

@straight-shoota
Copy link
Member

Okay this looks reasonable and not a specific issue with snap. So we should apply a generic solution.
The only reason for calling realpath on the crystal executable is being able to check whether it resolves to the wrapper script itself (to avoid self loops). It's not necessary for anything else and I think the script can be adapted to not use realpath for calling the crystal executable.

@straight-shoota
Copy link
Member

@koffeinfrei Could you check if this patch works for you?

--- a/bin/crystal
+++ b/bin/crystal
@@ -153,11 +153,12 @@ if [ -z "${PARENT_CRYSTAL##*/*}" -a "$(realpath "$PARENT_CRYSTAL")" = "$SCRIPT_P
   PARENT_CRYSTAL="crystal"
 fi

-PARENT_CRYSTAL_COMMAND=$(realpath "$(command -v "$PARENT_CRYSTAL" 2> /dev/null)" 2> /dev/null)
+PARENT_CRYSTAL_COMMAND=$(command -v "$PARENT_CRYSTAL" 2> /dev/null)
+PARENT_CRYSTAL_COMMAND_REALPATH=$(realpath "$PARNET_CRYSTAL_COMMAND" 2> /dev/null)
 PARENT_CRYSTAL_EXISTS=$(test !$?)

 # check if the parent crystal command refers to this script
-if [ "$PARENT_CRYSTAL_COMMAND" = "$SCRIPT_PATH" ]; then
+if [ "$PARENT_CRYSTAL_COMMAND_REALPATH" = "$SCRIPT_PATH" ]; then
   # remove the path to this script from PATH
   NEW_PATH="$(remove_path_item "$(remove_path_item "$PATH" "$SCRIPT_ROOT")" "bin")"
   # if the PATH did not change it will lead to an infinite recursion => display error

@koffeinfrei
Copy link
Contributor Author

@straight-shoota The patch works for me 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants