Shards: Allow overriding dependencies

[paulcsmith]

It would be helpful to be able to manually override a dependency, especially with how quickly Crystal changes.

Let's say for example that I have a dependency called taco_shard and it relies on another dependency called cheese_shard. Everything works but then a new version of Crystal makes a change that breaks cheese_shard. cheese_shard is no longer maintained so you fork it to make changes to make it work with the new Crystal. The problem is that you don't have a way to easily override that dependency without also forking taco_shard. It would be awesome to be able to just add something to shard.yml that overrides the version/resolver:

cheese_shard:
  github: myfork/cheese_shard
  override: true # say that it is ok to override what `taco_shard` specified

So even if taco_shard says it should use original/cheese_shard my shard.yml would override that and use the specific fork. This could also work for versions.

Prior art

• override option in Elixir mix: https://hexdocs.pm/mix/Mix.Tasks.Deps.html#module-dependency-definition-options (search for override)

[ysbaddaden]

To be honest, I really don't know. I can understand the need, but it can lead to confusion if a nested dependency overrides the shard source. It can also be very complex, and lead to more conflicts, what if shard A overrides B but a shard C also overrides B?

[RX14]

@ysbaddaden perhaps overrides could only be specified in the top-level shard.yml, which makes them far less confusing. This would them just be a "get my project to build" while taco_shard - which is presumably maintained - switches or forks cheese_shard.

I've really wanted something like this - but for path dependencies. Sometimes I want to hack on some transitive dependency, and it'd be great to just override it temporarily with a path, before committing back upstream.

[ysbaddaden]

@RX14 that sounds reasonable.

There are still edge cases: shall we error out when a library overrides a dependency? We should, but... what if an application that can be used as a library overrides a dependency?

It should be possible to implement it without too much work, by memorizing the override for a dependency (from the top-level spec only) then skipping the origin uniqueness validation when override is set.

[bcardiff]

Related to #105

So far the only (partial) workaround is to change the lib directory manually, but that can be done after a full shards installation to bring all nested dependencies (as in test-ecosystem).

Yet due to some breacking-changes that might not even be possible if post_install tasks fails to build. So, definitely +1 to allow overriding dependencies.

[RX14]

what if an application that can be used as a library overrides a dependency?

I'd worry about that when the issue appears in the issue tracker. Loosening constraints is free.

[paulcsmith]

Just another note about this causing issues. We typically run a lucky_cli build against lucky master. But sometimes other dependencies are set to a specific version. This caused an issue with 0.34.0 and the latest shards here: https://travis-ci.org/github/luckyframework/lucky_cli/jobs/671388658#L576

lucky:
  github: luckyframework/lucky
  branch: master
authentic:
  github: luckyframework/authentic
  version: 0.5.1

Error from shards install

Unable to satisfy the following requirements:
578  - `lucky *` required by `shard.yml`
579  - `lucky ~> 0.18.0` required by `authentic 0.5.1`

I guess shards treats branch: master as any version, which makes sense, but since authentic requires a specific version of lucky shards install fails. This makes sense, but in this case, I want to just override it and say: just use lucky master and override what authentic wants. Instead I'd have to go to authentic, set the lucky version to use the master branch, and then make a branch and change lucky_cli to use that branch...and repeat for any other shards that depends on a specific version of Lucky (which there are a few).

It would be so handy to just set override: true. Sure this may cause issues, but if it also solves a ton of issues, especially as shards and crystal change in these early days so much. And if it does cause issues override doesn't make it worse, you'd have to fork or change the shard somehow either way.

To set this as a priority I'd like to offer a bounty. This would be sooo helpful. I hope it can get in sometime soon :D

https://www.bountysource.com/issues/81762570-allow-overriding-dependencies

[bcardiff]

I feel the pain (in every test-ecosystem iteration).

I think is important to allow using forks and support patch development of nested dependencies without requiring to fork all the chain of dependencies. After a patch is found the whole release cycle of the chaing of dependencies should happen of course.

There are some risks in allowing such overrides in any shard.yml. In bundler repo there are similar discussion that didn't lead to something actionable. But bundle allows some limited local overrides to support this use case.

Allowing local development overrides as proposed #105, can also enable CI to run against master of different dependencies. The semantic of having a dependency in a shard.override.yml (?) would be that any constraint of a shard with the same name along the whole chain should be replaced with the one expressed in this file.

That at least would be my favorite way to solve it.

[RX14]

I'm happy just allowing people to set override: true in a dependency in the top-level shard.yml and that has the semantic of ignoring all other version constraints in the dependency tree and using the version constraint there.

[straight-shoota]

This is supposed to be only a development tool, right? override: true shouldn't be in a release. So it would be another candidate for a warning when this flag is present in a dependency. Even if it is ignored everywhere but the main shard.yml. and shards validate should fail when override: true is present.

[Sija]

@straight-shoota I wouldn't say it's a development tool per se, since you might wanna use it in production too but for sure override shouldn't be present in your app's dependencies - only in the top-level as @RX14 mentioned.

[straight-shoota]

Yeah, I meant it shouldn't propagate upwards in the dependecy tree. So you shouldn't release your shard with it and when override: true ends up in a release, it is to be ignored and considered a smell.

[RX14]

warning when this flag is present in a dependency

Agreed, and ignore it. Should only count in top-level shard.yml.

It's fine as a "oh god stuff's broken lets hotfix it" in production though

[bcardiff]

I think is best to have another file to define the overrides.

• Having it side by side, not changing the main shard.yml should be more clear that the overrides are local to the project.
• It can easily be ignored in the commit if it's something temporally used.
• It allows having an override checked in for ci that can not affect the main development workflow.
• It can be generated automatically easily. This might help tools like dependabot to be built.

As docker-compose does, we can have a convention like shard.override.yml, or alternative we can add a cli option to specify which override file to consider.

[Sija]

• Having it side by side, not changing the main shard.yml should be more clear that the overrides are local to the project.

Having another file with different structure seems odd.

• It can easily be ignored in the commit if it's something temporally used.

Committing the change with override: true also can be ignored and you don't wan't to ignore it altogether since usually you want to commit it for the main app, see previous comments.

[RX14]

I'm mixed bout the overrides file. I see the benefit to having it seperate, but it seems to make it more easy to accidentally commit code which only works in the presence of a local overrides file, which people could gitignore without considering the consequences.

I'm not convinced by the CI or the automatic generation usecase either - I can't imagine when these usecases apply.

[bcardiff]

I think that a comment in the shard.lock file says that it was generated with overrides is enough signalling to avoid missed commits. And if they are done, there will be clear information about that.

I'm not convinced by the CI or the automatic generation usecase either - I can't imagine when these usecases apply.

Having that simplifies how to keep a shard working against head versions of the dependencies. Otherwise there is a lot of burden to the maintainer to upon releasing, set the specific requirements of the top dependencies.

[RX14]

Having that simplifies how to keep a shard working against head versions of the dependencies

ah I see!

Yeah, I'm fine with a seperate file. Especially if we don't gitignore it by default (we'll leave people to shoot themselves in the foot with that)

[paulcsmith]

I'm a bit confused by the extra file not being committed. I think overriding is equally useful in production for when you have an app and you have two dependencies and one is using an outdated version of a shared dependency. Being able to override in that case should be committed and part of the app.

For that reason and others mentioned above I'd prefer an override: true that only works in the top-level and warns about subdependencies using override (and ignores it). I also like shard.yml having it because it is one less file to create and your dependencies are in on spot. You may be debugging and looking at shard.yml and wonder why it is looking for a version that isn't specificed in shard.yml

Maybe shard.override.yml is an optional thing? Also I think if we do add it, it should not be ignored. For example in my use-case above I'd want to override the dependency until we're ready to release a new version, not just locally, but for others and for CI. Also because of sometimes needing overrides in production apps.

Thoughts?

[bcardiff]

Also because of sometimes needing overrides in production apps.

If you want to make a release with a fork then I think is fine to push the developer to do a whole release of forked dependencies. It encourages more controlled dependencies I think.

In the scenario that doing a shards install with a shard.override.yml present, leaves a # WARNING: shard.override.yml used to determine dependencies in the shard.lock file you are still able to deploy that in production. But I think it will be fragile.

I think that disregarding version constraints should be scoped to checking if things will work after the update, or working on a patch for it.

---

In a situation were frameworkX depends on shardFoo and I want to patch shardFoo for my app built using frameworkX, maybe I need to temporalily fork framerworkX too, to use the fork of shardFoo. Deploy the app with the shard.lock.yml with warning is doable, but I would not make it the top use case.

[paulcsmith]

@bcardiff I think that can be quite hard sometimes. Asking people to fork dependencies (and potentially dependencies of dependencies) can be a big ask. Yeah they should, but in the early days of Elixir it was often the case that dependencies would go out of date and the author wouldn't update it. Then you'd have to fork it or find a fork of it, and it became a really big mess. They have an override: true that works pretty much like you'd expect. Overrides any of the other versions.

It is a bit of a hack, but sometimes you need those escape hatches. Most languages provide this, php, elixir, node (because you can do all kinds of stuff, too much IMO). I think bundler is the odd on out in this regard.

With that said, I'd be happy with anything 😂 if shard.override.yml is the way to do it it is better than what we have to do now :P. I'd still recommend checking it in by default though, otherwise I think it will be very confusing for the team collaborating on the code. They may see it in shard.lock, but they won't know what the override was or be able to easily modify it

[RX14]

I'm a bit confused by the extra file not being committed.

There's nothing stopping you from committing it, in fact I would also commit it and use it just the same as you to fix something which broke in production in a hurry.

[paulcsmith]

Ah ok, for some reason I thought this was going to be added to .gitignore by default. That was my bad 👍 I still prefer having all deps in one spot with override: true but would be happy to have override too is that is deemed the best. Both are better than now so that's great 🎉

[jwoertink]

Not sure if this falls in to the same category,but I ran in to an issue earlier, and I wasn't too sure how to best solve it.

I have a shard called "dj" that requires Avram. The latest version of Avram was 0.13.0, but I needed to test against a separate branch of Avram to see if a particular PR was going to work. So what I did was set "dj" to that avram PR branch, and then ran shards update, but got this error:

Unable to satisfy the following requirements:

- `avram *` required by `dj 0.4.1`
- `avram ~> 0.13.0` required by `lucky 0.20.0`
Failed to resolve dependencies

Would having an override: true option here be something that would have fixed this?

[Sija]

@jwoertink Yes, AFAIU if defined at the top-level of your app, it would _override_ the version constraint set by _lucky_ (and _dj_), effectively forcing shards to use whatever version you'd give.

[jwoertink]

Just wanted to come back to this. Working on an update for Lucky to fix an issue @bcardiff ran in to. I want to run specs against a Lucky project that is pointed to Lucky master, but it requires the Authentic shard which is pinned to Lucky ~> 0.22. My option currently are release another version of Authentic just to point to Lucky master, or drop the Lucky requirement down to 0.22.

 Installing shards
  Resolving dependencies
  Fetching https://github.com/luckyframework/lucky.git
  Fetching https://github.com/luckyframework/authentic.git
  Unable to satisfy the following requirements:

  - `lucky (branch master)` required by `shard.yml`
  - `lucky (~> 0.22.0)` required by `authentic 0.6.0`
  Failed to resolve dependencies

So having a way to tell Authentic it's ok to use the master branch version would be amazing.

[paulcsmith]

Yeah this has been a huge source of friction when updating Lucky since all the dependencies also need to be updated. @jwoertink what I did was create a branch and then point all the shards to that branch. Each shard also has its own branch.

This is a horrible explanation, but its the best way I could come up with for now