crystal-lang / shards

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

Shards overrides #422

Closed bcardiff closed 4 years ago

bcardiff commented 4 years ago

Implements most of #412 .

Reads a shard.override.yml (or the file specified by SHARDS_OVERRIDE) to override the source and restriction of a dependency. There are no changes regarding CLI options of shards.

This allows the cases described in the motivation of #412


Usage sample

On a project that uses github:crystal-lang/crystal-sqlite3 it will current use crysta-db 0.8.0.

# file: shard.yml
name: sample
version: 0.1.0

dependencies:
  sqlite3:
    github: crystal-lang/crystal-sqlite3
    version: ~> 0.15.0

crystal: 0.35.1
# file: shard.lock
version: 2.0
shards:
  db:
    git: https://github.com/crystal-lang/crystal-db.git
    version: 0.8.0

  sqlite3:
    git: https://github.com/crystal-lang/crystal-sqlite3.git
    version: 0.15.0

Let's suppose I need to work on a fix for crystal-db. I can create a shard.override.yml file pointing to my working copy of crystal-db

# file: shard.override.yml
dependencies:
  db:
    path: /my/path/to/crystal-db

Execute $ shards update to change the installed and locked version of crystal-db. (Note: if $ shards install is used instead of update, the use will get an error regarding that db has ambiguous sources due to the existing shard.lock and the shard.override.yml)

$ shards update
Resolving dependencies
Fetching https://github.com/crystal-lang/crystal-sqlite3.git
Installing db (0.9.0 at /my/path/to/crystal-db)
Using sqlite3 (0.15.0)
Writing shard.lock

After this update you will have a symlink to the /my/path/to/crystal-db

$ ls -la lib/
total 8
drwxr-xr-x   5 bcardiff  staff  160 Jul 22 11:15 .
drwxr-xr-x  15 bcardiff  staff  480 Jul 22 11:14 ..
-rw-r--r--   1 bcardiff  staff  196 Jul 22 11:15 .shards.info
lrwxr-xr-x   1 bcardiff  staff   43 Jul 22 11:15 db -> /my/path/to/crystal-db
drwxr-xr-x  12 bcardiff  staff  384 Jul 22 11:14 sqlite3

And the shard.lock has some comments to clarify if the lock was computed with overrides and will not be safe to commit.

# file: shard.lock
# NOTICE: This lockfile contains some overrides from shard.override.yml
version: 2.0
shards:
  db: # Overridden
    path: /my/path/to/crystal-db
    version: 0.9.0

  sqlite3:
    git: https://github.com/crystal-lang/crystal-sqlite3.git
    version: 0.15.0

Note that in this example we also bump crystal-db from 0.8 to 0.9 despite the fact that crystal-sqlite3 requires ~> 0.8.0. So we are overriding the requirements along the dependency graph.

You can override to a specific version, branch, fork, local path and solve ambiguos reference in case the ecosystem do not agree what is the main fork for a shard name.

Having a shard.master.yml will all dependencies to the master/develop branch will simplify how to check if you the project is up to date with non-released changes of dependencies.


Closes #412 Closes #105 Closes #299

jhass commented 4 years ago

Should we really touch the regular lock opposed to creating a shard.override.lock or not writing down the overrides at all?

If I have to take care to not commit the dirty lock, why have a separate file for overrides in the first place? It could just be section in shard.yml then, there's no difference to partially committing one or two files.

OTOH what's wrong with committing the overrides file and the lock changes from that? If collaborating on a project with outdated dependencies (the primary usecase for this), why should everybody have to redo the overrides?

Sija commented 4 years ago

This whole flow with additional file seems wrong to me. I'd rather take inspiration from Elixir (already mentioned) instead, KISS plz.

bcardiff commented 4 years ago

@jhass in theory there is nothing wrong with committing the override and lock with overrides. That should be done if the overrides are required for an app to work. It happens that I have more present the workflow of temporal overrides to fix things.

If the story for sharing overrides, or forcing some resolution without warnings is wanted the last proposal of the proposal in #412 can be implemented later:

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.

I think that having another lock file will generate confusion. The lock can be used as the source of truth for reinstalling the dependencies, if there are two, then there is a logic need to choose.

@Sija in #412 I mention that adding a forced or override attribute similar to Elixir one is not backward compatible and we will be forced to add a version in the shard.yml. The alternative is to override as in Yarn which I mention. Having an additional files allows per-user customization similar to Bundler which I've found convenient to work with custom forks and local paths.

mloughran commented 2 years ago

This is a very useful feature but it's currently not easy to find via google – I arrived here by following a trail of issues.

This may be related to the shards command page being outdated and not linked to the man page? /cc https://github.com/crystal-lang/crystal-book/issues/476