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

Manage compiler dependencies with shards #14363

Closed
straight-shoota opened this issue Mar 14, 2024 · 6 comments · Fixed by #14365
Closed

Manage compiler dependencies with shards #14363

straight-shoota opened this issue Mar 14, 2024 · 6 comments · Fixed by #14365

Comments

@straight-shoota
Copy link
Member

straight-shoota commented Mar 14, 2024

The compiler has two external dependencies vendored in at lib/: reply and markd.
To keep these synced with upstream, we use git subtree. Or at least that was the plan. It was never setup for lib/reply.
And while it's not terribly complex to setup and maintain, git subtree adds some overhead and it's not very straightforward to understand what's going on.

In the Crystal ecosystem we actually have a great tool for managing dependencies: shards!
Now of course the compiler should not require shards for building to avoid circular dependencies. But there's no need to. If we continue to keep the lib sources vendored in the repository, shards is only used for updating the sources. That would be a simple shards update then. Compare that with the commands for git subtree which I'm far from being able to recall from the top of my head. Of course, that could be automated. But the point stands that shards is just so much easier to use. And you have the full features available to check which versions are installed, check if they're outdated etc.
We should expect compiler dependencies to be published as Crystal shards, so this should work with any future additions.

So here's what I'm proposing:

  • Put a shard.yml into this repository with declared dependencies of reply and markd at the currently installed versions.
  • Run shards install and there should be no difference in the lib/ folder except a new lib/.shards.info file (which will also be checked in like the rest of lib/, and shard.lock as well)
@ysbaddaden
Copy link
Contributor

This is a huge 👍 from me. I guess I was too early when I proposed that 5 years ago.

Let's treat the compiler as a regular crystal application already.

@HertzDevil
Copy link
Contributor

Trying to do this with the current declared versions in the respective shard.ymls:

name: Crystal
version: 1.12.0-dev

authors:
  - Crystal Team <crystal@manas.tech>

dependencies:
  markd:
    github: icyleaf/markd
    version: "0.4.2"
  reply:
    github: I3oris/reply
    version: "0.3.0"

license: Apache-2.0

It looks like neither dependency matches exactly.

To avoid breaking existing code, we might as well update those dependencies to include at least these patches:

@straight-shoota
Copy link
Member Author

The currently installed version of markd is exactly icyleaf/markd@5e5a75d

reply is a bit weird as it does not match any commit in the main branch exactly. But I3oris/reply@90a7eb5 is functionally equivalent with no relevant changes.

@straight-shoota
Copy link
Member Author

straight-shoota commented Mar 15, 2024

@ysbaddaden I suppose the selling point would've been that we can just keep lib/ checked in.

@ysbaddaden
Copy link
Contributor

ysbaddaden commented Mar 15, 2024

I'm stubborn, maybe we'll drop the lib folder in another 5 years 😂

More seriously, both dependencies are external to crystal-lang so we'll probably want to ensure that they're vendored in to avoid any potential breakage.

@straight-shoota
Copy link
Member Author

Yeah, I've recently thought about whether checking in lib shouldn't be a general recommendation for any shard.
There's basically no real downside except adding a bit more code to the repository. But that's not a lot of memory. It's just a snapshot of the dependency. Not its entire history. It allows to work on the code without needing to deal with shards except for updating dependencies. If a repo is unreachable, you still have all the code you need to build.

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

Successfully merging a pull request may close this issue.

3 participants