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

postinstall or other hook needed to build c library #484

Open
didactic-drunk opened this issue Mar 18, 2021 · 8 comments
Open

postinstall or other hook needed to build c library #484

didactic-drunk opened this issue Mar 18, 2021 · 8 comments
Labels

Comments

@didactic-drunk
Copy link

I have a shard with no dependencies that builds a c library through postinstall. Unfortunately postinstall never runs from shards install when using the shard directly.

This is causing minor pain in 2 areas:

  1. Other developers can't clone a project and start working without special instructions.
 git clone project
 shards install
 crystal spec # FAILS: c library missing
  1. Ci needs special steps or the library is never built.

Solution: an additional hook that always runs or run postinstall after installing other dependencies in the same order as the shard being listed as a dependency.

If neither solution is acceptable then:

  • Why should there be an extra step when postinstall is automatic for deps?
  • Why should postinstall be special when listed as a dep but not for the shard itself?
  • It's bad UX and inconsistent.

In #305 @ysbaddaden indicates 1 hook should do everything and #319 postinstall is the appropriate hook for building non crystal dependencies. If so then the missing postinstall call is probably an oversight.

@straight-shoota
Copy link
Member

I agree the postinstall hook should be run from the main shard.yml as well, not just from dependencies. I think that's missing is just a bug.

@straight-shoota
Copy link
Member

straight-shoota commented Jul 2, 2021

I don't fully agree to my previous comment anymore.

The reason is: Shards is a dependency resolver. Not a build system or package installer.

If you install a project for local building, you don’t do that via shards (git clone project in the OP). And you don't run specs via shards (crystal spec in the OP).
Shards is only used to install the dependencies. In that capacity, it can also set up the dependencies via postinstall hooks. Maybe that's already questionable, I'm not sure. But setting up the environment for the current project is different. It seems more like a build step rather than a dependency install.

The solution to both problems mentioned in the OP is to use a build tool. I think make is great for this and I use it for all my shards. You check out a repo, run make test and it sets up the local build environment (shards install, build binary dependencies, setup database, etc.) and runs the spec suite. All in one command.
It offers lots of additional goodies, like updating dependencies when shard.yml gets changed.

I use this Makefile as a basis for any new Crystal project: https://gist.github.com/straight-shoota/275685fcb8187062208c0871318c4a23

@didactic-drunk
Copy link
Author

I thought make isn't necessarily available on windows. A single command to make the shard ready for use is still desirable to reduce developer friction.

@straight-shoota
Copy link
Member

You can use cmake on windows. However, you'll probably need different build commands on windows and posix systems anyways.

@didactic-drunk
Copy link
Author

didactic-drunk commented Jul 2, 2021

A single universal command that i don't need to read documentation for each shard to to figure out what to run is still desirable to reduce developer friction.

Pros:

  • You don't need to read docs to figure out what to run. Always type shards install
  • I can use shell aliases. sbi
  • The command is consistent between platforms. (Less to memorize)
  • If I hook this to another cross platform build system the command is identical and doesn't need special cases
  • I'm sure there are more reasons

@straight-shoota
Copy link
Member

straight-shoota commented Jul 2, 2021

Sounds great, but I don't think it's realistic to implement.

If you know any truly cross-platform build system, where you can run a single universal command on any system and it just sets everything up for you as necessary, please use just that. I'm sure it can even run shards install for you.

Or, in other words: I don't think we can or even should implement such a universal build system in shards. It's just completely out of scope, an entire, complex problem space on its own. We should stay with the Unix philosophy to do only one thing, and do that right.

However, I would be happy to consider integrating external solutions for this problem with shards. But for doing this, we need to have options available. I don't know any existing system that could do what you're describing. At least none that is publicly available and easy to use. That's a good reason why not to try doing this in shards: it's very hard.
But feel free to make any suggestions and we can talk about how this could work out with shards.

@didactic-drunk
Copy link
Author

If you know any truly cross-platform build system, where you can run a single universal command on any system and it just sets everything up for you as necessary, please use just that. I'm sure it can even run shards install for you.

I think there is a misunderstanding. shards install is the universal build command which runs platform dependent commands specified in shard.yml.

  • shards install is the only thing a developer needs to know to get up and running
  • shards install can be used with shell aliases
  • shards install works on any platform
  • shards install can be hooked in to other build or ci scripts/systems

I outlined a very simple proposal #507. It's probably less than 10 lines of code.

Let's see

  generic_platform = "posix" or "windows" # depending on environment
  install_cmd = case postinstall = shard_yaml["postinstall"]
    when String
      postinstall
    when Hash
      postinstall[platform_with_arch]? || postinstall["#{platform}-default"]? || 
      postinstall[generic_platform_with_arch]? || postinstall["#{generic_platform}-default"]? ||
      postinstall["default"]? ||
      raise "couldn't locate platform for #{platform_with_arch}"
    else
      raise "unknown type #{postinstall}, wanted String | Hash(String, String)"
    end

Slightly over budget but not too complex I hope.

@straight-shoota
Copy link
Member

That's by far not what I would consider a universal build command. It just multiplexes completely different commands based on platform. And as I said in #507 (comment) this kind of thing can easily be moved to a script. This doesn't need to be provided by shards at all. A custom script is also much more flexible for any specific use case.
The most that shards might have to provide is multiplexing between windows and posix platforms, because they require different kinds of scripts.

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

Successfully merging a pull request may close this issue.

2 participants