From 37ba1465965c5535bbcc0910fe493edd0fb958e2 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Wed, 18 Aug 2021 13:11:06 +0200 Subject: [PATCH] Reinstate "forbids" property (#781) * Load "forbids" property, and test * Update catalog spec document * doc/user-changes.md: documented "forbids" property --- doc/catalog-format-spec.md | 24 +++++++++ doc/user-changes.md | 17 +++++++ src/alire/alire-crates.adb | 1 + src/alire/alire-externals.adb | 1 + src/alire/alire-releases.adb | 10 +++- src/alire/alire-toml_load.adb | 51 +++++++++++++++---- src/alire/alire-toml_load.ads | 18 ++++--- .../solver/equivalences-conflict/test.py | 27 ---------- .../crate_conflict/crate_conflict-1.2.3.toml | 13 +++++ .../cr/crate_real/crate_real-1.0.0.toml | 9 ++++ .../cr/crate_subst/crate_subst-1.0.0.toml | 16 ++++++ .../tests/solver/forbids-replace/test.py | 35 +++++++++++++ .../solver/forbids-replace}/test.yaml | 0 testsuite/tests/solver/forbids/test.py | 33 ++++++++++++ testsuite/tests/solver/forbids/test.yaml | 4 ++ 15 files changed, 211 insertions(+), 48 deletions(-) delete mode 100644 testsuite/disabled/solver/equivalences-conflict/test.py create mode 100644 testsuite/fixtures/toolchain_index/cr/crate_conflict/crate_conflict-1.2.3.toml create mode 100644 testsuite/fixtures/toolchain_index/cr/crate_real/crate_real-1.0.0.toml create mode 100644 testsuite/fixtures/toolchain_index/cr/crate_subst/crate_subst-1.0.0.toml create mode 100644 testsuite/tests/solver/forbids-replace/test.py rename testsuite/{disabled/solver/equivalences-conflict => tests/solver/forbids-replace}/test.yaml (100%) create mode 100644 testsuite/tests/solver/forbids/test.py create mode 100644 testsuite/tests/solver/forbids/test.yaml diff --git a/doc/catalog-format-spec.md b/doc/catalog-format-spec.md index 11ddcba7..b98cdef7 100644 --- a/doc/catalog-format-spec.md +++ b/doc/catalog-format-spec.md @@ -524,6 +524,30 @@ static, i.e. they cannot depend on the context. # A crate depending on `bar^1` might find this `foo` release in its solution instead. ``` + - `forbids`: an array of tables containing dependency specifications, just as + the `depends-on` property. Releases matching one of the forbidden + dependencies are prevented from appearing in a solution with the release + doing the forbidding. + + There are two use cases for this property: + + 1. To codify known conflicts between releases for some reason (for example, + sources with the same name). + 2. To provide drop-in replacements for another crate, in conjunction with + a `provides` field. In this case the release must both provide and forbid + the crate for which it is a replacement. + + Example: + + ```toml + name = "bar" + version = "1.0" + provides = [ "foo=1.0" ] + [[forbids]] + baz = "*" # This crate cannot coexist with ours for some reason + foo = "*" # No other crate that provides foo is needed/allowed at the same time + ``` + ## Work-in-progress dependency overrides It is usual to develop several interdependent crates at the same time. In this scenario, it is often impractical to rely on indexed releases which are not intended to be modified. Instead, one would prefer to use a work-in-progress version of a crate to fulfill some dependency. diff --git a/doc/user-changes.md b/doc/user-changes.md index 89a89893..c6861c8d 100644 --- a/doc/user-changes.md +++ b/doc/user-changes.md @@ -6,6 +6,23 @@ stay on top of `alr` new features. ## Release `1.1` +### Conflicting releases + +PR [#781](https://github.com/alire-project/alire/pull/781) + +For releases that have known incompatibilities (duplicated source names, +drop-in equivalent crates), it is now possible to express this information +through a `forbids` table array, with the same syntax as dependencies. For +example: + +``` +[[forbids]] +conflicting_crate = "^1" +``` + +Releases related by a `forbids` property will not appear simultaneously as +dependencies in a solution, as the solver will discard these combinations. + ### Toolchain management PR [#775](https://github.com/alire-project/alire/pull/775) diff --git a/src/alire/alire-crates.adb b/src/alire/alire-crates.adb index 48b73034..43436108 100644 --- a/src/alire/alire-crates.adb +++ b/src/alire/alire-crates.adb @@ -173,6 +173,7 @@ package body Alire.Crates is Props => Properties, Deps => Unused_Deps, Equiv => Unused_Equiv, + Forbids => Unused_Deps, Pins => Unused_Pins, Avail => Unused_Avail); diff --git a/src/alire/alire-externals.adb b/src/alire/alire-externals.adb index 1a811bee..44b75de2 100644 --- a/src/alire/alire-externals.adb +++ b/src/alire/alire-externals.adb @@ -110,6 +110,7 @@ package body Alire.Externals is Props => Ext.Properties, Deps => Unused_Deps, Equiv => Unused_Equiv, + Forbids => Unused_Deps, Pins => Unused_Pins, Avail => Ext.Available); diff --git a/src/alire/alire-releases.adb b/src/alire/alire-releases.adb index c9718cc2..6311e38c 100644 --- a/src/alire/alire-releases.adb +++ b/src/alire/alire-releases.adb @@ -838,6 +838,7 @@ package body Alire.Releases is Props => This.Properties, Deps => This.Dependencies, Equiv => This.Equivalences, + Forbids => This.Forbidden, Pins => This.Pins, Avail => This.Available); @@ -949,9 +950,14 @@ package body Alire.Releases is end; end if; - -- Forbidden + -- Forbidden, wrapped as an array if not R.Forbidden.Is_Empty then - Root.Set (TOML_Keys.Forbidden, R.Forbidden.To_TOML); + declare + Forbid_Array : constant TOML.TOML_Value := TOML.Create_Array; + begin + Forbid_Array.Append (R.Forbidden.To_TOML); + Root.Set (TOML_Keys.Forbidden, Forbid_Array); + end; end if; -- Available diff --git a/src/alire/alire-toml_load.adb b/src/alire/alire-toml_load.adb index 67f0bcde..881588ad 100644 --- a/src/alire/alire-toml_load.adb +++ b/src/alire/alire-toml_load.adb @@ -39,6 +39,7 @@ package body Alire.TOML_Load is type Tables is (Available, Dependencies, + Forbids, Provides, Origin); @@ -69,23 +70,26 @@ package body Alire.TOML_Load is -- Load_Crate_Section -- ------------------------ - procedure Load_Crate_Section (Strict : Boolean; - Section : Crates.Sections; - From : TOML_Adapters.Key_Queue; - Props : in out Conditional.Properties; - Deps : in out Conditional.Dependencies; - Equiv : in out Alire.Provides.Equivalences; - Pins : in out User_Pins.Maps.Map; - Avail : in out Conditional.Availability) + procedure Load_Crate_Section + (Strict : Boolean; + Section : Crates.Sections; + From : TOML_Adapters.Key_Queue; + Props : in out Conditional.Properties; + Deps : in out Conditional.Dependencies; + Equiv : in out Alire.Provides.Equivalences; + Forbids : in out Conditional.Forbidden_Dependencies; + Pins : in out User_Pins.Maps.Map; + Avail : in out Conditional.Availability) is pragma Unreferenced (Pins); use TOML; use type Conditional.Dependencies; use type Conditional.Properties; - TOML_Avail : TOML.TOML_Value; - TOML_Deps : TOML.TOML_Value; - TOML_Equiv : TOML.TOML_Value; + TOML_Avail : TOML.TOML_Value; + TOML_Deps : TOML.TOML_Value; + TOML_Equiv : TOML.TOML_Value; + TOML_Forbids : TOML.TOML_Value; begin @@ -129,6 +133,31 @@ package body Alire.TOML_Load is & TOML_Keys.Depends_On); end if; + -- Process Forbids + + if Allowed_Tables (Section, TOML_Load.Forbids) then + if From.Pop (TOML_Keys.Forbidden, TOML_Forbids) then + From.Assert + (TOML_Forbids.Kind = TOML_Array, + "dependencies must be specified as array of tables"); + + for I in 1 .. TOML_Forbids.Length loop + Forbids := Forbids and + Dependency_Loader.Load + (From => From.Descend + (Key => TOML_Keys.Forbidden, + Value => TOML_Forbids.Item (I), + Context => "(group" & I'Img & ")"), + Loader => Conditional.Deps_From_TOML'Access, + Resolve => True, + Strict => Strict); + end loop; + end if; + elsif From.Unwrap.Has (TOML_Keys.Forbidden) then + From.Checked_Error ("found field not allowed in manifest section: " + & TOML_Keys.Forbidden); + end if; + -- Process Provides if Allowed_Tables (Section, Provides) then diff --git a/src/alire/alire-toml_load.ads b/src/alire/alire-toml_load.ads index 37b52e45..8d51b74c 100644 --- a/src/alire/alire-toml_load.ads +++ b/src/alire/alire-toml_load.ads @@ -18,14 +18,16 @@ package Alire.TOML_Load is function Load_File (File_Name : Any_Path) return TOML.TOML_Value; -- Will raise Checked_Error if file contents aren't valid TOML - procedure Load_Crate_Section (Strict : Boolean; - Section : Crates.Sections; - From : TOML_Adapters.Key_Queue; - Props : in out Conditional.Properties; - Deps : in out Conditional.Dependencies; - Equiv : in out Alire.Provides.Equivalences; - Pins : in out User_Pins.Maps.Map; - Avail : in out Conditional.Availability); + procedure Load_Crate_Section + (Strict : Boolean; + Section : Crates.Sections; + From : TOML_Adapters.Key_Queue; + Props : in out Conditional.Properties; + Deps : in out Conditional.Dependencies; + Equiv : in out Alire.Provides.Equivalences; + Forbids : in out Conditional.Forbidden_Dependencies; + Pins : in out User_Pins.Maps.Map; + Avail : in out Conditional.Availability); -- Loads parts of a manifest, taking into account if we are loading -- an indexed release, a local release, an external shared section or -- an external private section. diff --git a/testsuite/disabled/solver/equivalences-conflict/test.py b/testsuite/disabled/solver/equivalences-conflict/test.py deleted file mode 100644 index 9d51bc1c..00000000 --- a/testsuite/disabled/solver/equivalences-conflict/test.py +++ /dev/null @@ -1,27 +0,0 @@ -""" -Test that two crates providing the same third crate are incompatible -DISABLED because this is no longer conflicting. To be revisited when "forbids" -""" - -import subprocess -import os - -from drivers.alr import run_alr, init_local_crate, alr_with -from drivers.asserts import assert_eq, assert_match, match_solution -from re import escape as e - -# This test relies on two crates in the index: -# crate_equiv=2.0 also provides crate_virtual=1.0 -# crate_clash=1.0 also provides crate_virtual=1.0 -# Depending on the two of them cannot be solved, as that would mean two -# implementations of crate_virtual=1.0 at the same time - -init_local_crate("xxx") -alr_with("crate_equiv") -alr_with("crate_clash") - -match_solution("crate_equiv* (direct,missed)", escape=True) -# Because of alphabetical order, crate_clash is accepted first, and crate_equiv -# can no longer be accepted in the solution. - -print('SUCCESS') diff --git a/testsuite/fixtures/toolchain_index/cr/crate_conflict/crate_conflict-1.2.3.toml b/testsuite/fixtures/toolchain_index/cr/crate_conflict/crate_conflict-1.2.3.toml new file mode 100644 index 00000000..46f3b7f0 --- /dev/null +++ b/testsuite/fixtures/toolchain_index/cr/crate_conflict/crate_conflict-1.2.3.toml @@ -0,0 +1,13 @@ +description = "A crate that conflicts with other crates" +name = "crate_conflict" +version = "1.2.3" +maintainers = ["alejandro@mosteo.com"] +maintainers-logins = ["mylogin"] + +[[forbids]] +crate_virtual = "*" +crate_lone = "*" + +[origin] +url = "file:../../../crates/libhello_1.0.0.tgz" +hashes = ["sha512:99fa3a55540d0655c87605b54af732f76a8a363015f183b06e98aa91e54c0e69397872718c5c16f436dd6de0fba506dc50c66d34a0e5c61fb63cb01fa22f35ac"] diff --git a/testsuite/fixtures/toolchain_index/cr/crate_real/crate_real-1.0.0.toml b/testsuite/fixtures/toolchain_index/cr/crate_real/crate_real-1.0.0.toml new file mode 100644 index 00000000..4bfc56aa --- /dev/null +++ b/testsuite/fixtures/toolchain_index/cr/crate_real/crate_real-1.0.0.toml @@ -0,0 +1,9 @@ +description = "A crate" +name = "crate_real" +version = "1.0.0" +maintainers = ["alejandro@mosteo.com"] +maintainers-logins = ["mylogin"] + +[origin] +url = "file:../../../crates/libhello_1.0.0.tgz" +hashes = ["sha512:99fa3a55540d0655c87605b54af732f76a8a363015f183b06e98aa91e54c0e69397872718c5c16f436dd6de0fba506dc50c66d34a0e5c61fb63cb01fa22f35ac"] diff --git a/testsuite/fixtures/toolchain_index/cr/crate_subst/crate_subst-1.0.0.toml b/testsuite/fixtures/toolchain_index/cr/crate_subst/crate_subst-1.0.0.toml new file mode 100644 index 00000000..0c19b95a --- /dev/null +++ b/testsuite/fixtures/toolchain_index/cr/crate_subst/crate_subst-1.0.0.toml @@ -0,0 +1,16 @@ +description = "A crate that replaces another, forbidding other implementations" +name = "crate_subst" +version = "1.0.0" +maintainers = ["alejandro@mosteo.com"] +maintainers-logins = ["mylogin"] + +provides = [ "crate_real=1.0.0" ] + +[[forbids]] +crate_real = "*" +# This forbids means that no other crate should solve for crate_lone, as +# crate_subst is providing it already. + +[origin] +url = "file:../../../crates/libhello_1.0.0.tgz" +hashes = ["sha512:99fa3a55540d0655c87605b54af732f76a8a363015f183b06e98aa91e54c0e69397872718c5c16f436dd6de0fba506dc50c66d34a0e5c61fb63cb01fa22f35ac"] diff --git a/testsuite/tests/solver/forbids-replace/test.py b/testsuite/tests/solver/forbids-replace/test.py new file mode 100644 index 00000000..f11c6e4e --- /dev/null +++ b/testsuite/tests/solver/forbids-replace/test.py @@ -0,0 +1,35 @@ +""" +Test the usual use case in which a drop-in replacement crate forbids equivalent +crates in the solution. This works by both providing and forbidding the same +crate, which incidentally is the same modus operandi of apt-get "conflicts": +https://www.debian.org/doc/manuals/debian-reference/ch02.en.html#_package_dependencies +""" + +import subprocess +import os + +from drivers.alr import run_alr, init_local_crate, alr_with +from drivers.asserts import assert_eq, assert_match, match_solution +from re import escape as e + +# This test relies on two crates in the toolchain_index: +# crate_subst both provides and forbids crate_real +# crate_real is a regular crate only provided by itself and crate_subst + +# The following has only one possible solution, which is for crate_subst +# providing both dependencies. +init_local_crate("test") +alr_with("crate_real") + +# Check that this is initially solved with the regular crate. This is currently +# guaranteed by the solver attempting crates in alphabetical order. We will +# need eventually a way to disable equivalences (via pins, or solver config). +match_solution("crate_real=1.0.0 (origin: filesystem)", escape=True) + +# Let's add the drop-in equivalent crate that provides+forbids crate_lone +alr_with("crate_subst") +match_solution("crate_real=1.0.0 (crate_subst) (origin: filesystem)", + escape=True) # This is the substituted release +match_solution("crate_subst=1.0.0 (origin: filesystem)", escape=True) + +print('SUCCESS') diff --git a/testsuite/disabled/solver/equivalences-conflict/test.yaml b/testsuite/tests/solver/forbids-replace/test.yaml similarity index 100% rename from testsuite/disabled/solver/equivalences-conflict/test.yaml rename to testsuite/tests/solver/forbids-replace/test.yaml diff --git a/testsuite/tests/solver/forbids/test.py b/testsuite/tests/solver/forbids/test.py new file mode 100644 index 00000000..e21cdf43 --- /dev/null +++ b/testsuite/tests/solver/forbids/test.py @@ -0,0 +1,33 @@ +""" +Test that the forbids field works for regular and provided crates +""" + +import subprocess +import os + +from drivers.alr import run_alr, init_local_crate, alr_with +from drivers.asserts import assert_eq, assert_match, match_solution +from re import escape as e + +# This test relies on three crates in the toolchain_index: +# crate_conflict=1.2.3 conflicts with crate_lone* and crate_virtual* +# crate_lone is a regular crate +# crate_virtual has no releases, but is provided by crate_conflict_1 +# Crate conflict cannot appear with any of the others in a solution, because of +# its [forbids] table. + +init_local_crate("conflict_lone") +alr_with("crate_conflict") +alr_with("crate_lone") +match_solution("crate_(conflict|lone)=.* \(origin:.*\)") # has origin: solved +match_solution("crate_(conflict|lone)\* \(direct,missed\)") +# Because of load/solving details, we do not know which of the two crates is +# going to be missed/accepted in the solution, so we check there is one of each + +init_local_crate("conflict_virtual") +alr_with("crate_conflict") +alr_with("crate_virtual") +match_solution("crate_(conflict|virtual)=.* \(origin:.*\)") +match_solution("crate_(conflict|virtual)\* \(direct,missed\)") + +print('SUCCESS') diff --git a/testsuite/tests/solver/forbids/test.yaml b/testsuite/tests/solver/forbids/test.yaml new file mode 100644 index 00000000..8185c03b --- /dev/null +++ b/testsuite/tests/solver/forbids/test.yaml @@ -0,0 +1,4 @@ +driver: python-script +indexes: + toolchain_index: + in_fixtures: true -- 2.39.5