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

Feature/warn only crystal version #496

Merged
merged 10 commits into from
Apr 21, 2021
27 changes: 15 additions & 12 deletions docs/shard.yml.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -79,29 +79,32 @@ authors:
A restriction to indicate which are the supported crystal versions. This will
usually express a lower and upper-bound constraints (string, recommended)
+
When resolving dependencies, only the versions that comply with the current
crystal will be candidates. You can pass _--ignore-crystal-version_ to disregard this
behavior.
When resolving dependencies, this information is not used. After dependecies
have been determined shards will check all of them are expected to work with
the current crystal version. If not, a warning will appear for the ofending
packages. Yet you are able to keep use the resolved package version at your
beta-ziliani marked this conversation as resolved.
Show resolved Hide resolved
own risk.
+
The valid values are the same as for _dependencies.version_:
The valid values are mostly the same as for _dependencies.version_:
+
--
* It may be a version number.
* It may be _"*"_ if any version will do.
* The version number may be prefixed by an operator: _<_, _\<=_, _>_, _>=_ or _~>_.
* A version number prefixed by an operator: _<_, _\<=_, _>_, _>=_ or _~>_.
* Just _"*"_ if any version will do (this is the default if unspecified).
* Multiple requirements can be separated by commas.
--
There is a special legacy behavior (its use is discouraged) when just a version
number is used as the value: it works exactly the same as a `>=` check:
_x.y.z_ will be interpreted as _">= x.y.z"_
beta-ziliani marked this conversation as resolved.
Show resolved Hide resolved
+
When just a version number is used it has a different semantic compared to dependencies:
_x.y.z_ will be interpreted as _~> x.y, >= x.y.z_ (ie: _>= x.y.z, < (x+1).0.0_) honoring semver.
+
NOTE: If this property is not included it is treated as _< 1.0.0_.
You are welcome to also specify the upper bound to be lower than the next
(future) major Crystal version, because there's no guarantee that it won't
break your library.
+
*Example:*
+
[source,yaml]
----
crystal: ">= 0.35.0"
crystal: ">= 0.35, < 2.0"
----

*dependencies*::
Expand Down
5 changes: 1 addition & 4 deletions docs/shards.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,6 @@ after a command.
--local::
Don't update remote repositories, use the local cache only.

--ignore-crystal-version::
Do not enforce crystal version restrictions on shards.

-q, --quiet::
Decreases the log verbosity, printing only warnings and errors.

Expand All @@ -147,7 +144,7 @@ should be added to *PATH*.

SHARDS_OPTS::
Allows general options to be passed in as environment variable.
*Example*: _SHARDS_OPTS="--ignore-crystal-version" shards update_
*Example*: _SHARDS_OPTS="--no-color" shards update_

SHARDS_CACHE_PATH::
Defines the cache location. In this folder, shards stores local copies of remote
Expand Down
53 changes: 6 additions & 47 deletions spec/integration/install_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -939,44 +939,21 @@ describe "install" do
end
end

it "install version according to current crystal version" do
it "install latest version despite current crystal being older version, but warn" do
metadata = {dependencies: {incompatible: "*"}}
with_shard(metadata) do
run "shards install", env: {"CRYSTAL_VERSION" => "0.3.0"}
assert_installed "incompatible", "0.2.0"
end
end

it "install version according to current crystal version (major-minor only)" do
metadata = {dependencies: {incompatible: "*"}}
with_shard(metadata) do
run "shards install", env: {"CRYSTAL_VERSION" => "0.4.1"}
assert_installed "incompatible", "0.3.0"
end
end

it "install version ignoring current crystal version if --ignore-crystal-version" do
metadata = {dependencies: {incompatible: "*"}}
with_shard(metadata) do
stdout = run "shards install --ignore-crystal-version --no-color", env: {"CRYSTAL_VERSION" => "0.3.0"}
stdout = run "shards install --no-color", env: {"CRYSTAL_VERSION" => "0.3.0"}
assert_installed "incompatible", "1.0.0"
stdout.should contain(%(Shard "incompatible" may be incompatible with Crystal 0.3.0))
stdout.should contain(%(W: Shard "incompatible" may be incompatible with Crystal 0.3.0))
end
end

it "install version ignoring current crystal version if --ignore-crystal-version (via SHARDS_OPTS)" do
it "install latest version despite current crystal being newer version, but warn" do
metadata = {dependencies: {incompatible: "*"}}
with_shard(metadata) do
run "shards install", env: {"CRYSTAL_VERSION" => "0.3.0", "SHARDS_OPTS" => "--ignore-crystal-version"}
stdout = run "shards install --no-color", env: {"CRYSTAL_VERSION" => "2.0.0"}
assert_installed "incompatible", "1.0.0"
end
end

it "doesn't match crystal version for major upgrade" do
metadata = {dependencies: {incompatible: "*"}}
with_shard(metadata) do
ex = expect_raises(FailedCommand) { run "shards install --no-color", env: {"CRYSTAL_VERSION" => "2.0.0"} }
refute_installed "incompatible"
stdout.should contain(%(W: Shard "incompatible" may be incompatible with Crystal 2.0.0))
end
end

Expand All @@ -988,15 +965,6 @@ describe "install" do
end
end

it "warn about unneeded --ignore-crystal-version" do
metadata = {dependencies: {incompatible: "*"}}
with_shard(metadata) do
stdout = run "shards install --ignore-crystal-version --no-color", env: {"CRYSTAL_VERSION" => "1.1.0"}
assert_installed "incompatible", "1.0.0"
stdout.should contain(%(Using --ignore-crystal-version was not needed. All shards are already compatible with Crystal 1.1.0))
end
end

it "fails on conflicting sources" do
metadata = {dependencies: {
intermediate: "*",
Expand Down Expand Up @@ -1221,15 +1189,6 @@ describe "install" do
end
end

it "warn about unneeded --ignore-crystal-version" do
metadata = {dependencies: {incompatible: "*"}}
with_shard(metadata) do
stdout = run "shards install --ignore-crystal-version --no-color", env: {"CRYSTAL_VERSION" => "1.1.0"}
assert_installed "incompatible", "1.0.0"
stdout.should contain(%(Using --ignore-crystal-version was not needed. All shards are already compatible with Crystal 1.1.0))
end
end

describe "mtime" do
it "mtime lib > shard.lock > shard.yml" do
metadata = {dependencies: {
Expand Down
8 changes: 4 additions & 4 deletions spec/integration/spec_helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ private def setup_repositories
create_git_release "d", "0.2.0"

create_git_repository "incompatible"
create_git_release "incompatible", "0.1.0", {crystal: "0.1.0"}
create_git_release "incompatible", "0.2.0", {crystal: "0.2.0"}
create_git_release "incompatible", "0.3.0", {crystal: "0.4"}
create_git_release "incompatible", "1.0.0", {crystal: "1.0.0"}
create_git_release "incompatible", "0.1.0", {crystal: "~>0.1, >=0.1.0"}
create_git_release "incompatible", "0.2.0", {crystal: "~>0.2, >=0.2.0"}
create_git_release "incompatible", "0.3.0", {crystal: "~>0.4, >=0.4"}
create_git_release "incompatible", "1.0.0", {crystal: "~>1.0, >=1.0.0"}

create_git_repository "awesome"
create_git_release "awesome", "0.1.0", {
Expand Down
16 changes: 6 additions & 10 deletions src/cli.cr
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ module Shards
self.skip_postinstall = true
end
opts.on("--local", "Don't update remote repositories, use the local cache only.") { self.local = true }
opts.on("--ignore-crystal-version", "Do not enforce crystal version restrictions on shards.") { self.ignore_crystal_version = true }
straight-shoota marked this conversation as resolved.
Show resolved Hide resolved
opts.on("--ignore-crystal-version", "Kept for compatibility, to be removed in the future.") { }
beta-ziliani marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
opts.on("--ignore-crystal-version", "Kept for compatibility, to be removed in the future.") { }
opts.on("--ignore-crystal-version", "Has no effect. Kept for compatibility, to be removed in the future.") { }

opts.on("-v", "--verbose", "Increase the log verbosity, printing all debug statements.") { self.set_debug_log_level }
opts.on("-q", "--quiet", "Decrease the log verbosity, printing only warnings and errors.") { self.set_warning_log_level }
opts.on("-h", "--help", "Print usage synopsis.") { self.display_help_and_exit(opts) }
Expand All @@ -59,8 +59,7 @@ module Shards
Commands::Init.run(path)
when "install"
Commands::Install.run(
path,
ignore_crystal_version: self.ignore_crystal_version?
path
)
when "list"
Commands::List.run(path, tree: args.includes?("--tree"))
Expand All @@ -69,22 +68,19 @@ module Shards
path,
args[1..-1].reject(&.starts_with?("--")),
print: args.includes?("--print"),
update: args.includes?("--update"),
ignore_crystal_version: self.ignore_crystal_version?
update: args.includes?("--update")
)
when "outdated"
Commands::Outdated.run(
path,
prereleases: args.includes?("--pre"),
ignore_crystal_version: self.ignore_crystal_version?
prereleases: args.includes?("--pre")
)
when "prune"
Commands::Prune.run(path)
when "update"
Commands::Update.run(
path,
args[1..-1].reject(&.starts_with?("--")),
ignore_crystal_version: self.ignore_crystal_version?
args[1..-1].reject(&.starts_with?("--"))
)
when "version"
Commands::Version.run(args[1]? || path)
Expand Down Expand Up @@ -134,7 +130,7 @@ module Shards
begin
Commands::Check.run(path)
rescue
Commands::Install.run(path, ignore_crystal_version: self.ignore_crystal_version?)
Commands::Install.run(path)
end

Commands::Build.run(path, targets, options)
Expand Down
14 changes: 2 additions & 12 deletions src/commands/command.cr
Original file line number Diff line number Diff line change
Expand Up @@ -76,30 +76,20 @@ module Shards
def handle_resolver_errors
yield
rescue e : Molinillo::ResolverError
if e.is_a?(Molinillo::VersionConflict) && e.conflicts.has_key?(CrystalResolver.key)
suggestion = ", try updating incompatible shards or use --ignore-crystal-version as a workaround if no update is available."
end

Log.error { e.message }
raise Shards::Error.new("Failed to resolve dependencies#{suggestion}")
raise Shards::Error.new("Failed to resolve dependencies")
end

def check_ignored_crystal_version(packages)
def check_crystal_version(packages)
crystal_version = Shards::Version.new Shards.crystal_version

warned = false
packages.each do |package|
crystal_req = MolinilloSolver.crystal_version_req(package.spec)

if !Shards::Versions.matches?(crystal_version, crystal_req)
warned = true
Log.warn { "Shard \"#{package.name}\" may be incompatible with Crystal #{Shards.crystal_version}" }
end
end

unless warned
Log.warn { "Using --ignore-crystal-version was not needed. All shards are already compatible with Crystal #{Shards.crystal_version}" }
end
end

def touch_install_path
Expand Down
8 changes: 3 additions & 5 deletions src/commands/install.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ require "../molinillo_solver"
module Shards
module Commands
class Install < Command
def run(*, ignore_crystal_version = false)
def run
if Shards.frozen? && !lockfile?
raise Error.new("Missing shard.lock")
end

Log.info { "Resolving dependencies" }

solver = MolinilloSolver.new(spec, override, ignore_crystal_version: ignore_crystal_version)
solver = MolinilloSolver.new(spec, override)

if lockfile?
# install must be as conservative as possible:
Expand All @@ -38,9 +38,7 @@ module Shards
# Touch install path so its mtime is bigger than that of the lockfile
touch_install_path

if ignore_crystal_version
check_ignored_crystal_version(packages)
end
check_crystal_version(packages)
end

private def validate(packages)
Expand Down
4 changes: 2 additions & 2 deletions src/commands/lock.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ require "../molinillo_solver"
module Shards
module Commands
class Lock < Command
def run(shards : Array(String), print = false, update = false, *, ignore_crystal_version = false)
def run(shards : Array(String), print = false, update = false)
Log.info { "Resolving dependencies" }

solver = MolinilloSolver.new(spec, override, ignore_crystal_version: ignore_crystal_version)
solver = MolinilloSolver.new(spec, override)

if lockfile?
if update
Expand Down
4 changes: 2 additions & 2 deletions src/commands/outdated.cr
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ module Shards
@up_to_date = true
@output = IO::Memory.new

def run(@prereleases = false, *, ignore_crystal_version = false)
def run(@prereleases = false)
return unless has_dependencies?

Log.info { "Resolving dependencies" }

solver = MolinilloSolver.new(spec, override, prereleases: @prereleases, ignore_crystal_version: ignore_crystal_version)
solver = MolinilloSolver.new(spec, override, prereleases: @prereleases)
solver.prepare(development: Shards.with_development?)

packages = handle_resolver_errors { solver.solve }
Expand Down
8 changes: 3 additions & 5 deletions src/commands/update.cr
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ require "../molinillo_solver"
module Shards
module Commands
class Update < Command
def run(shards : Array(String), *, ignore_crystal_version = false)
def run(shards : Array(String))
Log.info { "Resolving dependencies" }

solver = MolinilloSolver.new(spec, override, ignore_crystal_version: ignore_crystal_version)
solver = MolinilloSolver.new(spec, override)

if lockfile? && !shards.empty?
# update selected dependencies to latest possible versions, but
Expand All @@ -30,9 +30,7 @@ module Shards
# Touch install path so its mtime is bigger than that of the lockfile
touch_install_path

if ignore_crystal_version
check_ignored_crystal_version(packages)
end
check_crystal_version(packages)
end

private def install(packages : Array(Package))
Expand Down
1 change: 0 additions & 1 deletion src/config.cr
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,5 @@ module Shards
class_property? frozen = false
class_property? with_development = true
class_property? local = false
class_property? ignore_crystal_version = false
class_property? skip_postinstall = false
end
18 changes: 5 additions & 13 deletions src/molinillo_solver.cr
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ module Shards
setter locks : Array(Package)?
@solution : Array(Package)?
@prereleases : Bool
@ignore_crystal_version : Bool

include Molinillo::SpecificationProvider(Shards::Dependency, Shards::Spec)
include Molinillo::UI

def initialize(@spec : Spec, @override : Override? = nil, *, prereleases = false, ignore_crystal_version = false)
def initialize(@spec : Spec, @override : Override? = nil, *, prereleases = false)
@prereleases = prereleases
@ignore_crystal_version = ignore_crystal_version
end

def prepare(@development = true)
Expand Down Expand Up @@ -172,25 +170,19 @@ module Shards
end

def dependencies_for(specification : S) : Array(R)
spec_dependencies = apply_overrides(specification.dependencies)

return spec_dependencies if specification.name == "crystal"
return spec_dependencies if @ignore_crystal_version

crystal_dependency = Dependency.new("crystal", CrystalResolver::INSTANCE, MolinilloSolver.crystal_version_req(specification))
spec_dependencies + [crystal_dependency]
apply_overrides(specification.dependencies)
end

def self.crystal_version_req(specification : Shards::Spec)
crystal_pattern =
if crystal_version = specification.crystal
if crystal_version =~ /^(\d+)\.(\d+)(\.(\d+))?$/
"~> #{$1}.#{$2}, >= #{crystal_version}"
if crystal_version =~ /^\d+\.\d+(\.\d+)?$/
">= #{crystal_version}"
else
crystal_version
end
else
"< 1.0.0"
"*"
end

VersionReq.new(crystal_pattern)
Expand Down