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

Add shards info command #324

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

straight-shoota
Copy link
Member

This is a basic implementation of #86

It adds a new shards info command which can display information about a shard. For now, the features are very crude. The main purpose is to provide a basis for crystal-lang/crystal#8792 to extract the shard name and version from the working directory.
Additional features can be added later.

@ysbaddaden
Copy link
Contributor

For starters, this is unrelated to #86 which would search & return information about any shard from a hypothetical registry (i.e. remote shards, not the local project).

For a local SPEC, the info is already right there, and the SPEC format is fully documented and stable. Why not read it directly?

About the docs use-case: having crystal docs depend on shards makes even less sense to me than having it depend on libyaml. The documentation for ruby gems don't print the gem name either, and it's fine.

@straight-shoota
Copy link
Member Author

straight-shoota commented Feb 17, 2020

Then #86 completely missed the point to illustrate it's about querying some remote index. I understand it as being primarily concerned with querying information about installed shards.

This PR only reflects the shard in current working directory, but an enhancement would be to receive a shard name which would be resolved according to local shard.yml.
Querying a remote index would be another possible enhancement. But IMO the core functionality would be to retrieve information about shards and dependencies in a local scope.

Since the shards system works without a central registry, the only real source of truth about features in the ecosystem is the locally available implementation. It is based on unique combination of shard.yml configurations with git tags (for the primary resolver). You can obviously put that all together yourself to find information. But I believe the shards program should offer a way to reliably query information about locally installed shards, their releases and dependencies. This kind of introspection is already available because it's used internally. It should also be exposed in an external interface to allow other tools (like the docs generator) to easily use this information.

This PR is obviously only a first step into reaching that goal.

having crystal docs depend on shards makes even less sense to me than having it depend on libyaml.

It's exactly the same reason why shards calls git commands instead of binding libgit. shards is most usually installed with the compiler. Also it's not a hard dependency. If shards command is not available, the doc generator can still work.
Any solution trying to do this without shards in the compiler means re-implementing at least parts of what is already solved in shards. So it seems like a good idea to re-use what's already there.

@Sija
Copy link
Contributor

Sija commented Feb 17, 2020

Similarly, RubyGems also have an info command, gem info pry here, fwiw:

*** LOCAL GEMS ***

pry (0.12.2)
    Authors: John Mair (banisterfiend), Conrad Irwin, Ryan Fitzgerald,
    Kyrylo Silin
    Homepage: http://pryrepl.org
    License: MIT
    Installed at: /usr/local/lib/ruby/gems/2.6.0

    An IRB alternative and runtime developer console

@straight-shoota
Copy link
Member Author

@Sija Yes, that might be worth pointing out. Most package/dependency managers I'm aware of have such a tool.

@ysbaddaden
Copy link
Contributor

#86 completely missed the point to illustrate it's about querying some remote index.

Huh? it could hardly be more explicit: "The info command asks the registry for informations about a package". It's even linked to #85 which itself links to #87. It also explicitly specifies a shard name in the command line.

It's exactly the same reason why shards calls git commands instead of binding libgit

Please don't make up historical background. Here is the actual reason:

Shards relies on the external git binary because it was much easier to deal with a few simple git commands which I knew very well, than creating bindings from scratch to an external C library which I knew nothing about. We're talking about a couple hours vs days, if not weeks, of work. Yet, that could change, see #129. I often have to install git for shards to run in containers for example, and always forget to. Having a self contained shards binary would be nice, but it would also make it more complex (and probably introduce another external shard).

Here is the truth: I hate unrequited complexity and adding things for whatever reason and no real benefit. In my eyes, this whole PR is merely:

YAML.parse("shard.yml")["name"]

I doubt this PR would exist if the SPEC was written in JSON. The "Crystal can't depend on libyaml" argument wouldn't exist anymore —which is already nonsensical to me since Shards already depends on libyaml and its "most usually installed with the compiler" anyway.

Note that the shards version command exists because it's easy to run from a macro —where we can't use the YAML bindings (nor JSON or whatever)— and solved an actual issue directly related to shards: to keep a single source of truth about a shard version.

Which means that this PR could become interesting. I just currently fail to grasp it.

@Sija AFAIK the Ruby gem's info command can query locally installed gems which are installed globally, as well as gems from the rubygems registry. This can be useful.

@asterite
Copy link
Member

Anyone up to porting libyaml to Crystal? At least the parser+scanner and emitter.

There's already a port in Go so my guess is that it shouldn't be hard. Plus it should end up being shorter because of Crystal's syntax, blocks, etc.

That way we can get rid of libyaml and also parse shards.yml directly in the compiler for many things (for example docs).

@straight-shoota
Copy link
Member Author

There's actually no indication that #86 is about a remote registry. Since shards is decentralized, that thought didn't suggest itself. My understanding of registry in this context was the collection of locally available shards, as they are referred to by name in shard.yml.
But anyway, that's just an exercise in misinterpretation. I mainly wanted to point out that the feature description is completely valid without querying any kind of remote instance.

Please don't make up historical background.

I just referred to #129 (comment) and I don't think anything, including your explanation here, contradicts my statement. Yes, we could change the compiler to use libyaml. But relying on shards binary is just easier by a lot. And it's also a good exercise in separating concerns.

I hate unrequited complexity and adding things for whatever reason and no real benefit.

I honestly see no nothing of that sort. On the contrary, it would add unnecessary complexity to the doc generator if it had to deal with reading shards spec. Even if that was just a one-liner to implement, it still carries a lot of baggage.

I tried to keep this PR focused on a small use case in order to merge quickly what currently blocks advancing the doc generator.
But maybe the general usefulness of this tool is easier acknowledge when looking at the broader scope. I've detailed this in the original issue in #86 (comment). When shards are not only addressed by their name, but a repo URL, it seems very useful to also have command to query the name of that shard which would be already pretty complex to do without a dedicated tool.

@asterite Having libyaml as an external dependency contributes to making it difficult to use in the compiler. We could also use a very simplistic implementation that's barely good enough to reliably parse name: and version: properties from shard.yml. But that's not the point. IMO the main reason to not implement parsing shard.yml in the compiler/doc generator is separation of concerns. shards is the tool for dealing with shard-specific features. There's no point in re-implementing anything of that when the shards binary could easily be enhanced to deliver such kind of introspection in a machine-readable way.

This change is necessary in order to allow using them as arguments of a
command which is intended for `shards info`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants