Skip to content

Dependency cli parsing #673

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.

Sign up for GitHub

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

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

Dependency cli parsing #673

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1e95757
Add parts_from_cli helper method to parse cli arguments
bcardiff May 16, 2025
aae1479
Allow Dependency to preserve resolver key and source
bcardiff May 16, 2025
eafca30
Extract logic from Dependency, update supported syntax
bcardiff May 16, 2025
040cbdd
single logic for known git providers
bcardiff May 16, 2025
53353dc
normalize paths from windows to posix
bcardiff May 16, 2025
91c3656
URI-based dependency parsing
straight-shoota May 20, 2025
47ec209
Fix windows CI
bcardiff May 21, 2025
ac81d3c
Fix windows CI (take 2)
bcardiff May 26, 2025
d03ae53
Update dependency_definition.cr
bcardiff May 26, 2025
05f3d97
Use # instead of @
bcardiff Jun 8, 2025
05483f2
Add # syntax to other source schemes
bcardiff Jun 8, 2025
47831a1
Make # be exact version
bcardiff Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions spec/unit/dependency_definition_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
require "./spec_helper"
require "../../src/dependency_definition"

private def expect_parses(value, resolver_key : String, source : String, requirement : Shards::Requirement)
Shards::DependencyDefinition.parts_from_cli(value).should eq(Shards::DependencyDefinition::Parts.new(resolver_key: resolver_key, source: source, requirement: requirement))
end

module Shards
describe DependencyDefinition do
it ".parts_from_cli" do
# GitHub short syntax
expect_parses("github:foo/bar", "github", "foo/bar", Any)
expect_parses("github:Foo/[email protected]", "github", "Foo/Bar", VersionReq.new("~> 1.2.3"))

# GitHub urls
expect_parses("https://github.com/foo/bar", "github", "foo/bar", Any)
Copy link
Member

@straight-shoota straight-shoota May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: I'm wondering about why this gets normalized to github resolver instead of "git", "https://github.com/foo/bar".
Both options are valid. So we only need to decide which one we want to pick.
I suppose the reason for github is that it's more concise? That's nice but not strictly necessary.
git would be closer to the original input.

Copy link
Member Author

@bcardiff bcardiff May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me closer to the intent is to have github: foo/bar, because I'm assuming the user is copy-pasting the url in the browser. We do preserve the format with trailing .git as those are copied from the clone popup.

At some point maybe it's worth configuring shards to map all github source to be resolved in some specific way, but is a separate story.


# GitHub urls from clone popup
expect_parses("https://github.com/foo/bar.git", "github", "foo/bar", Any)
expect_parses("[email protected]:foo/bar.git", "git", "[email protected]:foo/bar.git", Any)

# GitLab short syntax
expect_parses("gitlab:foo/bar", "gitlab", "foo/bar", Any)

# GitLab urls
expect_parses("https://gitlab.com/foo/bar", "gitlab", "foo/bar", Any)

# GitLab urls from clone popup
expect_parses("https://gitlab.com/foo/bar.git", "gitlab", "foo/bar", Any)
expect_parses("[email protected]:foo/bar.git", "git", "[email protected]:foo/bar.git", requirement: Any)

# Bitbucket short syntax
expect_parses("bitbucket:foo/bar", "bitbucket", "foo/bar", Any)

# bitbucket urls
expect_parses("https://bitbucket.com/foo/bar", "bitbucket", "foo/bar", Any)

# Git convenient syntax since resolver matches scheme
expect_parses("git://git.example.org/crystal-library.git", "git", "git://git.example.org/crystal-library.git", Any)
expect_parses("[email protected]:foo/bar.git", "git", "[email protected]:foo/bar.git", Any)

# Local paths
local_absolute = "/an/absolute/path"
local_relative = "an/relative/path"

# Path short syntax
expect_parses("./#{local_relative}", "path", "./#{local_relative}", Any)
expect_parses("../#{local_relative}", "path", "../#{local_relative}", Any)
expect_parses(".\\relative\\windows", "path", ".\\relative\\windows", Any)
expect_parses("..\\relative\\windows", "path", "..\\relative\\windows", Any)
# Path file schema
expect_parses("file://#{local_relative}", "path", local_relative, Any)
Copy link
Member

@straight-shoota straight-shoota May 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: This is technically incorrect. The URL file://a/releative/path means path /relative/path on host a. The // indicates an authority.

The correct URL for a local path would be file:a/relative/path.

I believe some applications implicitly interpret file://a/b/c as file:a/b/c because this is a common mistake.
The file scheme is not intended for remote access. We can consider doing that, but I don't think it's a good idea as it would solidify invalid behaviour.
Instead, the best way to handle this is probably to error if the host of a file URI is anything else but empty or localhost.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, we can send a warning about the preferred form, but still accept it.

Let's be nice with our users. I've used that format before myself.

expect_parses("file://#{local_absolute}", "path", local_absolute, Any)
# Path resolver syntax
expect_parses("path:#{local_absolute}", "path", local_absolute, Any)
expect_parses("path:#{local_relative}", "path", local_relative, Any)
# Other resolvers short
expect_parses("git:git://git.example.org/crystal-library.git", "git", "git://git.example.org/crystal-library.git", Any)
end
end
end
43 changes: 0 additions & 43 deletions spec/unit/dependency_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,49 +71,6 @@ module Shards
parse_dependency({foo: {git: "", tag: "rc-1.0"}}).to_s.should eq("foo (tag rc-1.0)")
parse_dependency({foo: {git: "", commit: "4478d8afe8c728f44b47d3582a270423cd7fc07d"}}).to_s.should eq("foo (commit 4478d8a)")
end

it ".parts_from_cli" do
# GitHub short syntax
Dependency.parts_from_cli("github:foo/bar").should eq({resolver_key: "github", source: "foo/bar", requirement: Any})
Dependency.parts_from_cli("github:Foo/[email protected]").should eq({resolver_key: "github", source: "Foo/Bar", requirement: VersionReq.new("~> 1.2.3")})

# GitHub urls
Dependency.parts_from_cli("https://github.com/foo/bar").should eq({resolver_key: "github", source: "foo/bar", requirement: Any})
Dependency.parts_from_cli("https://github.com/Foo/Bar/commit/000000").should eq({resolver_key: "github", source: "Foo/Bar", requirement: GitCommitRef.new("000000")})
Dependency.parts_from_cli("https://github.com/Foo/Bar/tree/v1.2.3").should eq({resolver_key: "github", source: "Foo/Bar", requirement: GitTagRef.new("v1.2.3")})
Dependency.parts_from_cli("https://github.com/Foo/Bar/tree/some/branch").should eq({resolver_key: "github", source: "Foo/Bar", requirement: GitBranchRef.new("some/branch")})

# GitLab short syntax
Dependency.parts_from_cli("gitlab:foo/bar").should eq({resolver_key: "gitlab", source: "foo/bar", requirement: Any})

# GitLab urls
Dependency.parts_from_cli("https://gitlab.com/foo/bar").should eq({resolver_key: "gitlab", source: "foo/bar", requirement: Any})

# Bitbucket short syntax
Dependency.parts_from_cli("bitbucket:foo/bar").should eq({resolver_key: "bitbucket", source: "foo/bar", requirement: Any})

# bitbucket urls
Dependency.parts_from_cli("https://bitbucket.com/foo/bar").should eq({resolver_key: "bitbucket", source: "foo/bar", requirement: Any})

# Git convenient syntax since resolver matches scheme
Dependency.parts_from_cli("git://git.example.org/crystal-library.git").should eq({resolver_key: "git", source: "git://git.example.org/crystal-library.git", requirement: Any})

# Local paths
local_absolute = File.join(tmp_path, "local")
local_relative = File.join("spec", ".repositories", "local") # rel_path is relative to integration spec
Dir.mkdir_p(local_absolute)

# Path short syntax
Dependency.parts_from_cli(local_absolute).should eq({resolver_key: "path", source: local_absolute, requirement: Any})
Dependency.parts_from_cli(local_relative).should eq({resolver_key: "path", source: local_relative, requirement: Any})

# Path resolver syntax
Dependency.parts_from_cli("path:#{local_absolute}").should eq({resolver_key: "path", source: local_absolute, requirement: Any})
Dependency.parts_from_cli("path:#{local_relative}").should eq({resolver_key: "path", source: local_relative, requirement: Any})

# Other resolvers short
Dependency.parts_from_cli("git:git://git.example.org/crystal-library.git").should eq({resolver_key: "git", source: "git://git.example.org/crystal-library.git", requirement: Any})
end
end
end

Expand Down
109 changes: 1 addition & 108 deletions src/dependency.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,105 +7,8 @@ module Shards
property name : String
property resolver : Resolver
property requirement : Requirement
# resolver's key and source are normalized. We preserve the key and source to be used
# in the shard.yml file in these field. This is used to generate the shard.yml file
# in a more human-readable way.
# A Dependency can still be created without them, but it will not be possible to
# generate the shard.yml file.
property! resolver_key : String
property! source : String

def initialize(@name : String, @resolver : Resolver, @requirement : Requirement = Any, @resolver_key : String? = nil, @source : String? = nil)
end

# Parse a dependency from a CLI argument
def self.from_cli(value : String) : Dependency
parts = parts_from_cli(value)

# We need to check the actual shard name to create a dependency.
# This requires getting the actual spec file from some matching version.
resolver = Resolver.find_resolver(parts[:resolver_key], "unknown", parts[:source])
version = resolver.versions_for(parts[:requirement]).first || raise Shards::Error.new("No versions found for dependency: #{value}")
spec = resolver.spec(version)
name = spec.name || raise Shards::Error.new("No name found for dependency: #{value}")

Dependency.new(name, resolver, parts[:requirement], parts[:resolver_key], parts[:source])
end

# :nodoc:
#
# Parse the dependency from a CLI argument
# and return the parts needed to create the proper dependency.
#
# Split to allow better unit testing.
def self.parts_from_cli(value : String) : {resolver_key: String, source: String, requirement: Requirement}
resolver_key = nil
source = ""
requirement = Any

if File.directory?(value)
resolver_key = "path"
source = value
end

if value.starts_with?("https://github.com")
resolver_key = "github"
uri = URI.parse(value)
source = uri.path[1..-1] # drop first "/""

components = source.split("/")
case components[2]?
when "commit"
source = "#{components[0]}/#{components[1]}"
requirement = GitCommitRef.new(components[3])
when "tree"
source = "#{components[0]}/#{components[1]}"
requirement = if components[3].starts_with?("v")
GitTagRef.new(components[3])
else
GitBranchRef.new(components[3..-1].join("/"))
end
end
end

if value.starts_with?("https://gitlab.com")
resolver_key = "gitlab"
uri = URI.parse(value)
source = uri.path[1..-1] # drop first "/""
end

if value.starts_with?("https://bitbucket.com")
resolver_key = "bitbucket"
uri = URI.parse(value)
source = uri.path[1..-1] # drop first "/""
end

if value.starts_with?("git://")
resolver_key = "git"
source = value
end

unless resolver_key
Resolver.resolver_keys.each do |key|
key_schema = "#{key}:"
if value.starts_with?(key_schema)
resolver_key = key
source = value.sub(key_schema, "")

# narrow down requirement
if source.includes?("@")
source, version = source.split("@")
requirement = VersionReq.new("~> #{version}")
end

break
end
end
end

raise Shards::Error.new("Invalid dependency format: #{value}") unless resolver_key

{resolver_key: resolver_key, source: source, requirement: requirement}
def initialize(@name : String, @resolver : Resolver, @requirement : Requirement = Any)
end

def self.from_yaml(pull : YAML::PullParser)
Expand Down Expand Up @@ -151,16 +54,6 @@ module Shards
end
end

# Used to generate the shard.yml file.
def to_shard_yaml(yaml : YAML::Builder)
yaml.scalar name
yaml.mapping do
yaml.scalar resolver_key
yaml.scalar source
requirement.to_yaml(yaml)
end
end

def as_package?
version =
case req = @requirement
Expand Down
114 changes: 114 additions & 0 deletions src/dependency_definition.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
require "./dependency"

module Shards
class DependencyDefinition
record Parts, resolver_key : String, source : String, requirement : Requirement

property dependency : Dependency
# resolver's key and source are normalized. We preserve the key and source to be used
# in the shard.yml file in these field. This is used to generate the shard.yml file
# in a more human-readable way.
property resolver_key : String
property source : String

def initialize(@dependency : Dependency, @resolver_key : String, @source : String)
end

# Used to generate the shard.yml file.
def to_yaml(yaml : YAML::Builder)
yaml.scalar dependency.name
yaml.mapping do
yaml.scalar resolver_key
yaml.scalar source
dependency.requirement.to_yaml(yaml)
end
end

# Parse a dependency from a CLI argument
def self.from_cli(value : String) : DependencyDefinition
parts = parts_from_cli(value)

# We need to check the actual shard name to create a dependency.
# This requires getting the actual spec file from some matching version.
resolver = Resolver.find_resolver(parts.resolver_key, "unknown", parts.source)
version = resolver.versions_for(parts.requirement).first || raise Shards::Error.new("No versions found for dependency: #{value}")
spec = resolver.spec(version)
name = spec.name || raise Shards::Error.new("No name found for dependency: #{value}")

DependencyDefinition.new(Dependency.new(name, resolver, parts.requirement), parts.resolver_key, parts.source)
end

# :nodoc:
#
# Parse the dependency from a CLI argument
# and return the parts needed to create the proper dependency.
#
# Split to allow better unit testing.
def self.parts_from_cli(value : String) : Parts
resolver_key = nil
source = ""
requirement = Any

if value.starts_with?("file://")
resolver_key = "path"
source = value[7..-1] # drop "file://"
end

# relative paths
if value.starts_with?("./") || value.starts_with?("../") || value.starts_with?(".\\") || value.starts_with?("..\\")
resolver_key = "path"
source = value
end

if value.starts_with?("https://github.com")
resolver_key = "github"
uri = URI.parse(value)
source = uri.path[1..-1].rchop(".git") # drop first "/""
end

if value.starts_with?("https://gitlab.com")
resolver_key = "gitlab"
uri = URI.parse(value)
source = uri.path[1..-1].rchop(".git") # drop first "/""
end

if value.starts_with?("https://bitbucket.com")
resolver_key = "bitbucket"
uri = URI.parse(value)
source = uri.path[1..-1] # drop first "/""
end

if value.starts_with?("git://")
resolver_key = "git"
source = value
end

if value.starts_with?("git@")
resolver_key = "git"
source = value
end

unless resolver_key
Resolver.resolver_keys.each do |key|
key_schema = "#{key}:"
if value.starts_with?(key_schema)
resolver_key = key
source = value.sub(key_schema, "")

# narrow down requirement
if source.includes?("@")
source, version = source.split("@")
requirement = VersionReq.new("~> #{version}")
end

break
end
end
end

raise Shards::Error.new("Invalid dependency format: #{value}") unless resolver_key

Parts.new(resolver_key: resolver_key, source: source, requirement: requirement)
end
end
end