crystal-lang / shards

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

RFC: Shards overrides #412

Closed bcardiff closed 4 years ago

bcardiff commented 4 years ago

Related to #105, #240, and #299

Motivation

There are stories that would benefit from forcing the resolution for a given shard.

Use local working copies

This is currently possible but requires the user to manually do symbolic links inside the lib folder. Yet shards provides a way to have path: dependencies. The user should be able to state that in the current machine a specific path can be used as the path source instead of the git one.

But we don't want to put these changes in shard.yml since it will pollute with something specific for the development environment.

Use a shard version despite published constraints

Maybe a shard foo was updated with a fix or feature we need in the v0.10.0 release. But another shard bar has a conservative constraint to foo of ~> 0.9.2. If an application requires both foo and bar the latest release v0.10.0 of foo is not a candidate.

This scenario currently push the user to fork bar. If there would be a need to actually fix bar, then it will be ok. But it might happen that the only need is to update the version constraint in it.

It would be desired to tweak and force the resolution of foo to a given version, despite the constraints of the rest of the dependencies. This only applies in the top level resolution. It is not desired for a dependency to force a version of another one.

Fixing a dependency

Frameworks have a medium graph size of dependencies. A bug might appear at any level.

In the following scenario with conservative constraints, if we find a bug on bar we will be forced to fork foo just to change the bar restriction.

framework
  shard.yml
    dependencies:
      foo:
        github: ...
        version: ~> 0.8.0

foo
  shard.yml
    dependencies:
      bar:
        github: ...
        version ~> 0.5.0

If I'm developing / contributing to framework it becomes a burden to fork foo in order to fix bar.

Some specs, specially on framework, will expand crystal projects and call the compiler within the test suite. Currently, if I’m trying to fix bar, I need to deal not only with forking foo, but to update the template used in the test suite.

Checking app against development version of a dependency

There is no easy way to keep a CI checking if the upcoming version of a dependency will work. Having an alternate shard.yml requires file renaming and also to deal with some complications mentioned in other scenarios.

At the moment, the way to accomplish this is for the shard to follow something like git-flow and use version restriction only in published releases, leaving branch master restriction on development. It's error prone and a burden that the community seems to prefer not to deal with.

Proposal

Allow shards to use as input not only shard.yml file but also

  1. a shard.override.yml in the same directory, and
  2. a user-wide shard.override.yml located in ~/.crystal/shards.override.yml, XDG_CONFIG_DIRS or similar directory.

Allow shards to accept -f <file> option to add additional override files into consideration.

Override files will have the same format as shard.yml, yet only dependencies: will be used. The semantic would be as follow:

If an override was used along the way, the shard.lock file will contain a warning comment indicating that it was generated with input information of specific files. This will help to prevent checking shard.lock files with local information, or at worst, recognizing them once checked in.

Usage

# ~/.crystal/shards.override.yml
dependencies:
  db:
    path: ~/path/to/crystal-db

Doing a shards install in crystal-mysql will create a symlink.

# shards.override.yml
dependencies:
  foo:
    github: non-main-author/foo

Whether I need to share that with others I can track shards.override.yml or not.

I consider allowing the main shard.yml file to have overrides like

# shard.yml 
dependencies:
  foo:
    github: non-main-author/foo
    forced: true
  bar:
    github: bar/bar

But introducing that will be backward incompatible, requiring to add a shard_version marker. Because if bar depends on foo as github: original-main-author/foo then there will be conflicts between which fork to use.

There is no reason to have a forced dependency on a library shard.yml. For development the prior proposal seems enough.

The only scenario where a forced dependency should be tracked is on application shard.yml. In this case, dealing with addition shard.override.yml seems like a reasonable compromise.

If we want overrides to be in the shard.yml we would need an overrides: or resolutions: sibling to dependencies:.

# shard.yml 
dependencies:
  bar:
    github: bar/bar
resolutions:
  foo:
    github: non-main-author/foo

This is similar to what is done in yarn. It would offer a slightly cleaner solution to the scenario where a team needs to share overrides. Here, there would be no need to add a warning in the shard.lock file. If wanted, this extension can be introduced later.

straight-shoota commented 4 years ago

Sounds good overall.

What exactly is the use case for a user-wide override file? This doesn't seem immediately necessary to me. I can't imagine a situation where I'd wanted to use the same overrides in every Crystal project. This seems quite intrusive and hard to follow. If you want to share the same overrides in related projects, you can just symlink shard.override.yml.

I'm also not sure about the -f semantics. IIUC correctly, the -f argument file and shard.override.yml are supposed to be merged with the former taking precedence? What if I want to use only -f argument file? Then I'd have to remove/rename shard.override.yml? Maybe it would be better to use -f argument file exclusively instead of shard.override.yml?

bcardiff commented 4 years ago

What exactly is the use case for a user-wide override file?

I would either have a user-wide or a lookup into parent path. The use case is that some frameworks specs expand crystal projects and run shards within them. Without an implicit override (user-wide or parent path) we can't override the created project in the spec. If we use parent path, then the expanded project will be forced to live under certain path and not for example /tmp.

I'm also not sure about the -f semantics.

I base the proposal in docker-compose, yet I didn't follow it exactly. I'm fine if providing -f turns off the lookup of shard.override.yml and other overrides.

jhass commented 4 years ago

Maybe a SHARD_OVERRIDES=/path/to/shards.overrides.yml can solve both, the user wide/framework and -f usecases?

straight-shoota commented 4 years ago

The use case is that some frameworks specs expand crystal projects and run shards within them. Without an implicit override (user-wide or parent path) we can't override the created project in the spec.

I don't follow how this "expand crystal projects" works. Can you give an example?

Regarding -f we could consider that you can pass this option multiple times and each instance is appended and overrides the previous ones. But only if there's an actual use case for compositing override files.

bcardiff commented 4 years ago

I'm satisfied if the only interface is via an environment variable.

I would prefer to be able to specify multiple files on the override though. I imagine that on a CI it might be useful to check upstream development versions of some shards individually and combined.

Regarding the "expand crystal projects" @straight-shoota , https://github.com/luckyframework/lucky_cli/blob/2e8c1ff767824a036aac7fc7d44ec30f9d1dcb5b/spec/integration/init_web_spec.cr#L28 is one. There are others that are harder to follow because the call to shards install are implicit in generators.

straight-shoota commented 4 years ago

So it's essentially just for a spec? Then I'd suggest the easiest solution would be to have that spec code use the main override file if such exists.

bcardiff commented 4 years ago

That would limit where the spec would need to expand the project + search on parent folders for the override file.

jwoertink commented 4 years ago

oh man... I could totally use this right now... I'm working on a new release for Lucky, but I'm stuck.

  Unable to satisfy the following requirements:

  - `habitat (branch jaw/v0.5.0)` required by `lucky 0.22.0+git.commit.04a3cfefa948d613a5a922739f43c1ebc2467c93`
  - `habitat (branch jaw/v0.5.0)` required by `authentic 0.6.0+git.commit.f18195a573f951b182db7b0041153ecc57bc8bc7`
  - `habitat (branch jaw/v0.5.0)` required by `carbon 0.1.1+git.commit.46e7ae26a440a26e1f50aa2f83b576d45cb1ebfd`
  - `habitat (branch jaw/v0.5.0)` required by `lucky_flow 0.6.3+git.commit.09e993f68f68476af3223b28da16268a99d7bf33`
  - `habitat (branch jaw/v0.5.0)` required by `avram 0.14.0+git.commit.5a7d7f2125f1206f5cd5633e9de30fb233100366`
  - `habitat (~> 0.4.3)` required by `webdrivers 0.3.0`
  Failed to resolve dependencies

We don't own the webdrivers shard, but our project LuckyFlow includes webdrivers as a dependency. So if we had a way to tell flow, to tell webdrivers, to tell habitat to just do what I want, that'd would be awesome.