search

crystal-lang / shards

Dependency manager for the Crystal language
Other
763 stars 100 forks source link

Allow CRYSTAL env var to determine compiler #415

Closed bcardiff closed 4 years ago

bcardiff commented 4 years ago

Fixes #406

Shards will read CRYSTAL env var, fallback to crystal, to determine which crystal compiler to use.

This also enables that a compiler+shards package can force to use the compiler of that same packaging if wanted.

Something that is not handled is how the executables + postinstall can deal with this override.

A dependency is forced to use in the postinstall a ${CRYSTAL:-"crystal"} instead of crystal which is not great. Another alternative is to encourage shards build instead of crystal build that way shards will be invoked again and can choose the appropiate crystal compiler since the environment is preserved. But we actually have the same problem, the postinstall assumes that shards is on the path. So what? we should have a ${SHARDS:-"shards"}? Not great.

I think that the scenario of having executable helpers in the bin directory that are compiled by dependencies on postinstall should be more declarative. That would be a way to solve it I think.

jhass commented 4 years ago

Can't we just figure out the directory $CRYSTAL lives in and prepend it to the $PATH before invoking postinstall? Or if we want to avoid whatever else might be in that directory to appear in $PATH, we could symlink $CRYSTAL to /tmp/something/crystal and push /tmp/something into $PATH.

bcardiff commented 4 years ago

I think that prepending current shards and crystal location to the path when invoking scripts from shards is acceptable.

jhass commented 4 years ago

I just realized, the symlink approach could have another advantage: it removes the need that $CRYSTAL points to something called crystal. It is not uncommon to deal with parallel installation of multiple versions of something by renaming some of the executables, so you would have for example /usr/bin/crystal and /usr/bin/crystal-1.2.

bcardiff commented 4 years ago

Good point. I usually have directories like crystal-x.y/bin/crystal. So I didn't consider it.

Should shards create & prepend the directory to the path only when needed? Identifying if the resolution to shards and crystal leads to the same executable that is running and to the Shards.crystal_bin location?

Those /usr/bin/crystal-1.2 that you and @straight-shoota seems to have are those symlinks? Or there are custom made wrapper scripts?

jhass commented 4 years ago

Should shards create & prepend the directory to the path only when needed? Identifying if the resolution to shards and crystal leads to the same executable that is running and to the Shards.crystal_bin location?

I think it's fine to start simple and make it smart afterwards. That is I'd be totally fine if any shards install invocation sets up the temp directory & symlink early and only cleans it up on process termination. Then iterative optimizations upon that can be to

Those /usr/bin/crystal-1.2 that you and @straight-shoota seems to have are those symlinks? Or there are custom made wrapper scripts?

I don't actually have that anywhere right now. It's just common to see from packages of other things, especially languages. I know for certain that Archlinux, Debian and Fedora/RHEL families of distributions install /usr/bin/ruby-2.6 or /usr/bin/ruby2.6 if you install a ruby2.6 package while the ruby package is say 2.7. Also it's basically ubiquitous to have /usr/bin/python, /usr/bin/python2 and /usr/bin/python3. Whether things are symlinks into /opt/, wrapper scripts that setup some environment variables (hello virtualenv, rbenv) or just executables with different standard paths compiled in shouldn't matter. Crystal would support all three modes of operation right now already, we should only make sure to preserve such here by simply making no assumptions about it. Symlinking a symlink is entirely fine.

bcardiff commented 4 years ago

Ok, will take the route of lazily setup only once the first postinstall is hit for now.

Something that might not work is if crystal-x.y/shards-x.y is an alias I think. They will not be found in path to append. $CRYSTAL will be forwarded but shards will not resolve to shards-x.y. This is out of my comfort zone 🙈 . It's fine if aliases are not supported, that why I wanted to check how the crystal-x.y was created (yet from the /usr/bin prefix it was obvious they are not aliases)

jhass commented 4 years ago

Well yes, aliases only exist in the current shell anyhow afaik, it won't even properly work to put them into $CRYSTAL.

straight-shoota commented 4 years ago

Well yes, aliases only exist in the current shell

They do work in subshells when configured via shellrc.

My two cents: I don't like where this is going. It adds even more build tool features to shards. Shards shouldn't be a build tool. It's a dependency manager. There are lots of build tools out there and most of them are better than shards for this job.

bcardiff commented 4 years ago

@straight-shoota how would you solve that the environment used is preserved along the way? Even if we assume that crystal is in the path, how we ensure the same shards is called in the postinstall?

bcardiff commented 4 years ago

Plus if CRYSTAL env var is used to choose the crystal compiler to grab the version for the resolution, how we can ensure that that binary is used as the compiler all along?

straight-shoota commented 4 years ago

how we ensure the same shards is called in the postinstall?

I'd let that be the responsibility of a real build tool.

jhass commented 4 years ago

So you would remove postinstall? Fact is that while people can just crystal build in a postinstall script, they will. I agree we shouldn't overboard, but I view this as the minimum effort we can make to not have that break in hilarious and hard to debug ways.

waj commented 4 years ago

I don't think having a postinstall hook makes Shards a build tool. I don't think it's just a dependency manager either. It's the tool that allows building, managing and (someday) publishing shards. Ideally many projects would just specify postinstall: make.

To me the noise is coming from the relationship with the targets spec. That is totally expendable and it would remove the need of exposing the shards binary in any way to the postinstall step. But I think the idea was always give an option to not require the users to deal with makefiles, bringing a default behaviour to call the Crystal compiler.

With the risk of turning this conversation away from the topic of this PR, maybe we could extend the postinstall syntax to integrate it better with targets and thus not requiring to specify a command that invokes shards. For example this could indicate that the target foo must be built after the shard is installed:

postinstall:
  target: foo

I may even suggest that the postinstall could be (optionally) an array, so more than one target or command can be specified. Some might be thinking "Oh no... MORE features", but this would provide more cohesion within the configuration.

If this message receive at least one thumbs up, I'll open a RFC so we could discuss the feature there 🙂

bcardiff commented 4 years ago

I will give the 👍 because I mention something like that in https://github.com/crystal-lang/shards/pull/415#issue-438750245

I think that the scenario of having executable helpers in the bin directory that are compiled by dependencies on postinstall should be more declarative. That would be a way to solve it I think.

Yet, that will not solve the issue that if a makefile is used either shards or crystal might be called there.

bcardiff commented 4 years ago

It seems that whatever solution we try to achieve here will lead to half-way implementation. Although #406 seems legit at first, shards is already bound to crystal binary in reality since post_install steps might call crystal compiler.

It was a nice excercise though :-)

For now it seems that the best things is to close this PR and maybe #406.

jhass commented 4 years ago

I didn't get what the outstanding issue with this approach is. Also let's not get the aim for a perfect solution to be in the way of useful iteration (that theme seems to expose in a couple of other places as well...). Once we find it, we still can move towards the perfect solution, improving status quo is not useless in the meantime.

bcardiff commented 4 years ago

Part of the line of thought was

406 proposed another way to determine which crystal version is wanted. Choosing the binary push the story of how to honor that choice within the postinstall steps.

If the postinstall where more declarative, not requiring to explicitly call crystal in a shell script there will be no need to set the PATH since shards could do that per target directly.

But users might still prefer to use make or other build tools and call crystal.

Maybe in the future we can have a standardize an env var like CC to choose the crystal compiler, that would remove the need for changing the PATH with temporal directories. It will also allow compilers named crystal-x.y to be used. Shards could set a fallback to that env var.

Part of my motivations here was to pave the road for a self-containted .tar.gz where the crystal and shards binary there will call each other. This will be better handle by wrapper scripts of crystal and shards that will prepend the ./bin location relative to the wrapper scripts to PATH.