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

Resolution of crystal dependency is hard tied to crystal executable #406

Closed
straight-shoota opened this issue Jun 8, 2020 · 4 comments · Fixed by #534
Closed

Resolution of crystal dependency is hard tied to crystal executable #406

straight-shoota opened this issue Jun 8, 2020 · 4 comments · Fixed by #534

Comments

@straight-shoota
Copy link
Member

The implicit crystal dependency introduced in #395 calls crystal env to determine the crystal version it should treat as a dependency.
It assumes you're installing dependencies for use with crystal binary in current PATH. But that might not at all be what I intend to do.
If I want to build with a compiler that's not crystal, shards might install wrong dependency versions when it implicitly uses crystal as reference. For example, I usually have a couple different compiler executables in my PATH. Currently, the versions are not very different, so it's probably not going to cause many problems. But as soon as we move on after 1.0, it would be quite common to have different compiler versions available, say crystal-1.0 and crystal-1.1 to test against the different releases.
Using --ignore-crystal-version would completely disable the crystal dependency and thus doesn't reproduce the same behaviour as when using a proper version.

Currently, you would have to change PATH to point to the appropriate executable in order for shards to pick the correct version as dependency. That requires much effort and can easily cause issues by itself. We need a better solution.

Ideas that come to mind are to specify either the crystal version directly or the path to the compiler that should be used as configuration option to shards.
A very neat solution would use the CRYSTAL enviornment value. That's typically used to make the compiler executable configurable in Makefiles. So it would integrate very well with that.

@bcardiff
Copy link
Member

bcardiff commented Jun 8, 2020

The CRYSTAL_VERSION environment variable can be used for now as a workaround. When set, the crystal env is not executed.

@waj
Copy link
Member

waj commented Jun 8, 2020

Using the CRYSTAL env var sounds reasonable. The order of precedence would be: CRYSTAL_VERSION to completely override the version, crystal executable at CRYSTAL and finally use just the Crystal compiler in the path.

@jhass
Copy link
Member

jhass commented Jun 9, 2020

I think this is not a too big issue, many tools will reference just crystal, requiring a path managing version manager like chruby for usecases like this. shards build is doing that for a long time already. That said I guess reading CRYSTAL for it certainly doesn't harm, let's make sure shards build uses it as well.

@straight-shoota
Copy link
Member Author

straight-shoota commented Nov 30, 2021

The discussion here and in #415 got derailed from the main point.

We need to focus on shards' own behaviour. It executes the crystal program as an important feature in the shards build command and to discover the Crystal version. There is currently no way to configure which crystal program you want to use. It's just picked up from PATH. Of course, you can configure your PATH but that requires a couple extra steps.

For Crystal version there is the environment variable CRYSTAL_VERSION, but there is no customizability for shards build.
There should be a simple option to determine which compiler program to use. Using the environment variable CRYSTAL for this is an established practice we're using in many build configurations in the Crystal project. I use it in every one of my shards. It's very useful to have this everywhere. Yet, shards as a major component of Crystal software projects, has a hard coded path an no direct way of configuring it.

I think the first couple commits from #415 are good and do all that we need.

Postinstall scripts won't pick up that configuration if they have crystal command hard coded. But that's their deal, not shards' problem to solve. If they're supposed to pick up alternative crystal locations, they should honour the CRYSTAL variable or do it some other way (that even works for SHARDS btw.).

shards must not bend over backwards to make things magically work in postinstall scripts. It just has to cover its own parts.

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