Merge the /bin/crystal Shell script in the compiler?

[j8r]

The /bin/crystal script is setting environment variables, then calling the compiler.

The questions are:

• is it a limitation of the language to not be able to write this script directly in Crystal?
• can't this environment variables like our other ones in our .bashrc or .zshrc?
• How about Windows, would it be in Powershell?

Issues:

• As noted by jemc on Gitter/IRC, this complicates compiler debugging with tools like lldb.
• This also render counter-intuitive to have multiple crystal - one may think to copy the shell script, which isn't going to work.

I think exposing CRYSTAL_PATH=$CRYSTAL_ROOT/src:lib, and potentially other variables, through an API that can be used by shards and other projects (ameba?) would be a nice plus, instead of assuming the library path in each project and hard-coding it.

[asterite]

This isn't a problem.

[j8r]

This is a problem for the Windows port, or we create bin/crystal.bat. Whatever the solution chosen, something has to be done.

[j8r]

Prevents also to set i_know_what_im_doing when we think we know what we are doing like #5835, but in fact not so much 😅

[straight-shoota]

I think we should keep bin/crystal as a developer tool. But I don't think the distribution packages should need it.

[asterite]

@straight-shoota Then who sets CRYSTAL_PATH to point it to the standard library?

[j8r]

For example look on Alpine, file $(which crystal) returns directly the binary.
docker run -it --rm -v $PWD:/app -w /app jrei/crystal-alpine sh -c 'ldd $(which crystal)'

        /lib/ld-musl-x86_64.so.1 (0x7f1d127c1000)
        libLLVM-5.0.so => /usr/lib/libLLVM-5.0.so (0x7f1d0f7d2000)
        libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x7f1d0f67d000)
        libpcre.so.1 => /usr/lib/libpcre.so.1 (0x7f1d0f620000)
        libgc.so.1 => /usr/lib/libgc.so.1 (0x7f1d0f5b1000)
        libevent-2.1.so.6 => /usr/lib/libevent-2.1.so.6 (0x7f1d0f566000)
        libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x7f1d0f552000)
        libc.musl-x86_64.so.1 => /lib/ld-musl-x86_64.so.1 (0x7f1d127c1000)
        libffi.so.6 => /usr/lib/libffi.so.6 (0x7f1d0f549000)
        libz.so.1 => /lib/libz.so.1 (0x7f1d0f52f000

Environments variables:

docker run -it --rm -v $PWD:/app -w /app jrei/crystal-alpine crystal env
CRYSTAL_CACHE_DIR="/root/.cache/crystal"
CRYSTAL_PATH="lib:/usr/lib/crystal/shards:/usr/lib/crystal/core"
CRYSTAL_VERSION="0.27.2"

So it's definitely possible. I've pushed commits in this way. Here the CI won't run, the draft PR being closed.

[straight-shoota]

The compiler can assign that itself. It would simply use the same default as the wrapper script, which is essentially #{File.dirname(File.dirname(File.real_path(PROGRAM_NAME)))}/src/:lib.

The wrapper script doesn't do anything else when used from the distribution package.

[straight-shoota]

@j8r The value for CRYSTAL_PATH comes from CRYSTAL_CONFIG_PATH which is set at compile time (see build step in APKBUILD). A hard coded path is not portable.

[j8r]

Or just src:lib @straight-shoota when either CRYSTAL_PATH or CRYSTAL_CONFIG_PATH aren't set.
Edit: this works only when developing the compiler, not when using the tarball archive. Your #{File.dirname(File.dirname(File.real_path(PROGRAM_NAME)))}/src/:lib is perfectly fine 👍

[RX14]

Distribution packages don't have to be portable, they know absolute paths so use CRYSTAL_CONFIG_PATH. The portable tar.gz uses a different bin/crystal to the one in this repo and needs to set CRYSTAL_PATH dynamically based on where it's run. The one in this repo needs the same since it needs to override the standard library location from the system location, and it needs additional logic to handle choosing a compiler.

Presumably if you're running gdb on a compiler, then you've built it using make crystal (otherwise the debugging information is likely not present, or it's an optimized binary) and so the correct CRYSTAL_PATH has already been compiled into the executable. So most of the pain is gone just by using gdb on .build/crystal

[j8r]

Ok, why @straight-shoota solution won't work? What prevents this logic to be in Crystal rather than in Shell, Python or Windows batch?

• Either we are an user, then using crystal thats takes the stdlib from ../src, from where the binary is located
• We develop the compiler, we use .build/crystal and the stdlib comes from again ../src. make use .build/crystal if present, else system's crystal

A wrapper doesn't look like being needed to solve this problem.

[straight-shoota]

@RX14 We could replace the crystal-wrapper fro distribution-scripts with a native crystal implementation as suggested in #7391 (comment). This is immediately portable and doesn't require a separate solution for windows. Having one less piece in the puzzle is generally an improvement as well.

@j8r

make use .build/crystal if present, else system's crystal

That's actually the primary job of bin/crystal. We shouldn't remove the development version from the repo. But the distribution version can be replaced.

[asterite]

What's the net benefit of doing this?

[j8r]

@asterite one less (possibly useless) file to support? (minus https://github.com/crystal-lang/distribution-scripts/commits/master/linux/files/crystal-wrapper).
It's also better to have less divergence between distributions. Some use this wrapper (tarball) , some don't (Alpine).
Another issue, the compiler is developed with a wrapper but the release is distributed with another one – not ideal.
The benefit is small, but the work is small too.
There are also confusion about this wrapper and the compiler binary.

[straight-shoota]

Reopening because this has come up again in #10702. I think it would be feasible and a good improvement to get rid of (the need for) the wrapper script in distribution. That is, not bin/crystal but https://github.com/crystal-lang/distribution-scripts/blob/8bc01e26291dc518390129e15df8f757d687871c/linux/files/crystal-wrapper
This needs compiler support for what I've mentioned in #7391 (comment) and #10702 (comment).