search

crystal-lang / shards

Dependency manager for the Crystal language
Other
765 stars 102 forks source link

Add subdirectory support for git dependencies #531

Closed matthewmcgarvey closed 2 years ago

matthewmcgarvey commented 2 years ago

Provides a way to load a shard that is a subdirectory in a git repo. Allows for the possibility of "monorepos" with multiple shards. I know this has been discussed a lot, though I can't find a related issue at the moment, but I don't believe I've seen anyone discuss this way of solving it. This follows a pattern used in other package managers, but the one I was most recently referenced was nim's package manager, nimble. To reference a subdirectory of a github repo, you add a query param to the url with a key of subdir and a value of the directory to use (?subdir=alpha).

Example

alpha:
  git: https://github.com/matthewmcgarvey/cronorepo.git?subdir=alpha
  branch: main

Technical

The way this works is by cloning the larger monorepo as normal, but then checking out the desired commit/branch/tag into a temp directory. From there, the subdirectory is copied over to the install path. I believe the temp directory is required because I did not find a way to checkout the ref into the install path directly with only the subdirectory and it be flattened as expected.

This subdir query param causes the clone to fail (I believe) so I also added code to remove it before cloning.

Other Considerations

This code is not clean, and I didn't intend for it to be. There doesn't seem to be consensus on whether or not things like this even should be done so I don't want to spend a bunch of time making the code perfect if it's going to be rejected on a foundational level.

This code only supports git urls. This is for many of the same reasons as above. If it's acceptable and merged, I would like to move on to considering what it might look like to include this in the github shorter version instead of the full url but I don't want to get ahead of myself.

Considered Alternative

I considered a different way of doing it that seems cleaner with our yaml files which was to have something like:

alpha:
  git: https://github.com/matthewmcgarvey/cronorepo.git
  branch: main
  subdir: alpha

But we currently only handle one Requirement and it's not passed to the Resolver anyways. I think a good refactor in the future would be to simply pass the Dependency into the resolver instead of pieces but I'm sure there's tradeoffs.

straight-shoota commented 2 years ago

This has previously been proposed and rejected in #238. I don't see how this PR does anything significantly differently or that any of the arguments has changed. The 1 shard = 1 repo doctrine is still standing strong.

I'm going to close this PR for now because we don't even need to look at any implementation until there's at least a reasonable consensus for adopting this. In order to reach that, I suggest to prepare a more elaborate RFC which summarizes the previous discussions, explains and disputes the arguments and makes a case why the previous decision should be revised. Maybe others who have showed interst in this feature can collaborate.

P.S. To help with getting started on a discourse: In my opinion, the strongest counter arguments are additional complexity, not just for shards but for all kind of tooling. 1 shard = 1 repo is just dead simple and easy to implement. And versioning is global to the repository. So you can't make individual releases for the components of a mono repo. That restricts usefulness vs. separate repositories and leaves separate dependencies as the major factor. Maybe there could be different solutions if that's a problem.