Skip to content

URI-based dependency parsing #678

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 12 additions & 1 deletion spec/unit/dependency_definition_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ module Shards
# bitbucket urls
expect_parses("https://bitbucket.com/foo/bar", "bitbucket", "foo/bar", Any)

# unknown https urls
expect_raises Shards::Error, "Cannot determine resolver for HTTPS URI" do
Shards::DependencyDefinition.parts_from_cli("https://example.com/foo/bar")
end

# 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("git@example.org:foo/bar.git", "git", "git@example.org:foo/bar.git", Any)
Expand All @@ -51,13 +56,19 @@ module Shards
expect_parses("..\\relative\\windows", "path", "../relative/windows", Any)
{% end %}
# Path file schema
expect_parses("file://#{local_relative}", "path", local_relative, Any)
expect_raises Shards::Error, "Invalid file URI" do
Shards::DependencyDefinition.parts_from_cli("file://#{local_relative}")
end
expect_parses("file:#{local_relative}", "path", local_relative, Any)
expect_parses("file:#{local_absolute}", "path", local_absolute, Any)
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)
expect_parses("git+https://example.org/foo/bar", "git", "https://example.org/foo/bar", Any)
expect_parses("git:https://example.org/foo/bar", "git", "https://example.org/foo/bar", Any)
Copy link
Contributor

Choose a reason for hiding this comment

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

There are no specs for mercurial/hg or fossil URL.

end
end
end
90 changes: 41 additions & 49 deletions src/dependency_definition.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ require "./dependency"

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

property dependency : Dependency
# resolver's key and source are normalized. We preserve the key and source to be used
Expand Down Expand Up @@ -45,59 +45,51 @@ module Shards
#
# 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
path = Path[value].to_posix.to_s
if path.starts_with?("./") || path.starts_with?("../")
resolver_key = "path"
source = path
end

uri = URI.parse(value)
if uri.scheme != "file" && uri.host &&
(resolver_key = GitResolver::KNOWN_PROVIDERS[uri.host]?)
source = uri.path[1..-1].rchop(".git") # 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
case scheme = uri.scheme
when Nil
case value
when .starts_with?("./"), .starts_with?("../")
Parts.new("path", Path[value].to_posix.to_s)
when .starts_with?("git@")
Copy link
Contributor

Choose a reason for hiding this comment

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

Edge case: it's the username to connect with, it doesn't have to be git.

Copy link
Member Author

@straight-shoota straight-shoota May 22, 2025

Choose a reason for hiding this comment

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

Yes of course. This is just an example for a heuristic to derive a resolver from a schemeless URI. If the username is git, it's likely a git repo.

We can do similar things for other URI components. For example, if the hostname starts with fossil., it's likely a fossil repo. If the path ends with .git it's likely a git repo.

I'm not entirely sure if we should do all those things when it's relatively simple to just provide a fully qualified URI. But it seems easy enough, shouldn't cause much issues and might improve UX in some cases.

Parts.new("git", value)
else
raise Shards::Error.new("Invalid dependency format: #{value}")
end
when "file"
raise Shards::Error.new("Invalid file URI: #{uri}") if !uri.host.in?(nil, "", "localhost") || uri.port || uri.user
Parts.new("path", uri.path)
when "https"
if resolver_key = GitResolver::KNOWN_PROVIDERS[uri.host]?
Parts.new(resolver_key, uri.path[1..-1].rchop(".git")) # drop first "/""
else
raise Shards::Error.new("Cannot determine resolver for HTTPS URI: #{value}")
end
when "git"
if uri.host
Parts.new("git", uri.to_s)
else
Parts.new("git", uri.path)
end
when "git+https"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why treat git particularly, instead of <resolver>+ for fossil and mercurial?

Edge cases: what about other schemes and resolver combinations, like git+file:, fossil+http:, or hg+ssh://?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, of course. That all sounds good. This patch was really just intended to demonstrate the concept as an alternative to the original approach in #673. It certainly isn't production ready.

uri.scheme = "https"
Parts.new("git", uri.to_s)
else
if resolver_class = Resolver::RESOLVER_CLASSES[scheme]?
uri.scheme = nil
source = uri.to_s
# narrow down requirement
requirement = Any
if source.includes?("@")
source, version = source.split("@")
requirement = VersionReq.new("~> #{version}")
end

return Parts.new(scheme, source, requirement)
end
raise Shards::Error.new("Invalid dependency format: #{value}")
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
6 changes: 1 addition & 5 deletions src/resolvers/resolver.cr
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,9 @@ module Shards
end

private record ResolverCacheKey, key : String, name : String, source : String
private RESOLVER_CLASSES = {} of String => Resolver.class
RESOLVER_CLASSES = {} of String => Resolver.class
private RESOLVER_CACHE = {} of ResolverCacheKey => Resolver

def self.resolver_keys
RESOLVER_CLASSES.keys
end

def self.register_resolver(key, resolver)
RESOLVER_CLASSES[key] = resolver
end
Expand Down
Loading