From aea88a8fe1303c4f51d962106caa0dcff921a341 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Tue, 18 Aug 2020 17:48:54 +0200 Subject: [PATCH] Remove fields from local manifest (#493) * Merge Alire.Releases.TOML_IO into Alire.Releases Now that Alire.Releases is no longer Preelaborable, there is no reason for the separation. * Do not emit/load `origin` for local manifests This information is only relevant for index manifests, so better not to confuse users with fields that shouldn't be there. * Make optional some fields in local manifests Fields related to packaging can be required at the time of `alr publish`, whereas they're not important for users not intending to package (yet). * Testsuite adjustments * Setup function for tests with init'ed crates * Testsuite: verify no `[origin]` in local manifests --- src/alire/alire-crates.ads | 10 ++- src/alire/alire-dependencies-states.adb | 6 +- src/alire/alire-origins.ads | 2 +- src/alire/alire-properties-from_toml.ads | 24 +++++-- src/alire/alire-releases-toml_io.adb | 26 ------- src/alire/alire-releases-toml_io.ads | 5 -- src/alire/alire-releases.adb | 68 ++++++++++++++++--- src/alire/alire-releases.ads | 38 +++++++---- src/alire/alire-toml_load.adb | 12 ++-- src/alire/alire-workspace.adb | 4 +- testsuite/drivers/alr.py | 12 ++++ testsuite/drivers/helpers.py | 10 ++- testsuite/tests/misc/local-no-origin/test.py | 27 ++++++++ .../tests/misc/local-no-origin/test.yaml | 3 + .../tests/misc/local-reject-origin/test.py | 19 ++++++ .../tests/misc/local-reject-origin/test.yaml | 1 + .../crates/crate_1234/alire/crate_1234.toml | 3 - .../tests/show/inside-vs-outside/test.py | 25 ++++++- 18 files changed, 218 insertions(+), 77 deletions(-) delete mode 100644 src/alire/alire-releases-toml_io.adb delete mode 100644 src/alire/alire-releases-toml_io.ads create mode 100644 testsuite/tests/misc/local-no-origin/test.py create mode 100644 testsuite/tests/misc/local-no-origin/test.yaml create mode 100644 testsuite/tests/misc/local-reject-origin/test.py create mode 100644 testsuite/tests/misc/local-reject-origin/test.yaml diff --git a/src/alire/alire-crates.ads b/src/alire/alire-crates.ads index 64b8308a..a5cd5942 100644 --- a/src/alire/alire-crates.ads +++ b/src/alire/alire-crates.ads @@ -15,8 +15,14 @@ package Alire.Crates is function Naming_Convention return Utils.String_Vector; -- Return a description of the naming restrictions on crates/indexes. - type Sections is (Release_Section, - -- Top-level table, with all info about a release + type Sections is (Index_Release, + -- Top-level table, with all info about a release to be + -- stored in an index. + + Local_Release, + -- Top-level table, with info for the working copy of a + -- release. This allows regular users not dealing with + -- packaging to see fewer fields. External_Shared_Section, -- Top-level table, with only info valid for externals diff --git a/src/alire/alire-dependencies-states.adb b/src/alire/alire-dependencies-states.adb index 10b88c7d..47d3f923 100644 --- a/src/alire/alire-dependencies-states.adb +++ b/src/alire/alire-dependencies-states.adb @@ -155,8 +155,10 @@ package body Alire.Dependencies.States is when Missed => null; when Solved => - Table.Set (Keys.Release, - This.Fulfilled.Release.Constant_Reference.To_TOML); + Table.Set + (Keys.Release, + This.Fulfilled.Release.Constant_Reference.To_TOML + (Manifest.Index)); end case; end To_TOML; diff --git a/src/alire/alire-origins.ads b/src/alire/alire-origins.ads index c408810d..70a12b9b 100644 --- a/src/alire/alire-origins.ads +++ b/src/alire/alire-origins.ads @@ -164,7 +164,7 @@ private function Packaged_As (Name : String) return Package_Names is (Name => +Name); - type Origin_Data (Kind : Kinds := Kinds'First) is record + type Origin_Data (Kind : Kinds := External) is record Hashes : Hash_Vectors.Vector; case Kind is diff --git a/src/alire/alire-properties-from_toml.ads b/src/alire/alire-properties-from_toml.ads index 08ee34a6..e217f2b1 100644 --- a/src/alire/alire-properties-from_toml.ads +++ b/src/alire/alire-properties-from_toml.ads @@ -48,13 +48,20 @@ package Alire.Properties.From_TOML is Name => True, others => False), - Crates.Release_Section => + Crates.Index_Release => (Description | Maintainers | Maintainers_Logins | Name | Version => True, - others => False)); + others => False), + + Crates.Local_Release => + (Description | + Name | + Version => True, + others => False) + ); type Loader_Array is array (Property_Keys range <>) of Property_Loader; @@ -140,9 +147,13 @@ package Alire.Properties.From_TOML is return Conditional.Properties is (Loader (From, External_Shared_Loaders, External_Shared_Section)); - function Release_Loader (From : TOML_Adapters.Key_Queue) - return Conditional.Properties is - (Loader (From, Release_Loaders, Release_Section)); + function Index_Release_Loader (From : TOML_Adapters.Key_Queue) + return Conditional.Properties is + (Loader (From, Release_Loaders, Index_Release)); + + function Local_Release_Loader (From : TOML_Adapters.Key_Queue) + return Conditional.Properties is + (Loader (From, Release_Loaders, Local_Release)); Section_Loaders : constant array (Crates.Sections) of access @@ -150,6 +161,7 @@ package Alire.Properties.From_TOML is return Conditional.Properties := (External_Private_Section => External_Private_Loader'Access, External_Shared_Section => External_Shared_Loader'Access, - Release_Section => Release_Loader'Access); + Index_Release => Index_Release_Loader'Access, + Local_Release => Local_Release_Loader'Access); end Alire.Properties.From_TOML; diff --git a/src/alire/alire-releases-toml_io.adb b/src/alire/alire-releases-toml_io.adb deleted file mode 100644 index 19674822..00000000 --- a/src/alire/alire-releases-toml_io.adb +++ /dev/null @@ -1,26 +0,0 @@ -with Ada.Text_IO; - -with TOML.File_IO; - -package body Alire.Releases.TOML_IO is - - ------------- - -- To_File -- - ------------- - - procedure To_File (R : Release; Filename : String) is - use Ada.Text_IO; - File : File_Type; - begin - Create (File, Out_File, Filename); - TOML.File_IO.Dump_To_File (R.To_TOML, File); - Close (File); - exception - when others => - if Is_Open (File) then - Close (File); - end if; - raise; - end To_File; - -end Alire.Releases.TOML_IO; diff --git a/src/alire/alire-releases-toml_io.ads b/src/alire/alire-releases-toml_io.ads deleted file mode 100644 index cf539ff2..00000000 --- a/src/alire/alire-releases-toml_io.ads +++ /dev/null @@ -1,5 +0,0 @@ -package Alire.Releases.TOML_IO is - - procedure To_File (R : Release; Filename : String); - -end Alire.Releases.TOML_IO; diff --git a/src/alire/alire-releases.adb b/src/alire/alire-releases.adb index db3df05b..747aedfe 100644 --- a/src/alire/alire-releases.adb +++ b/src/alire/alire-releases.adb @@ -1,4 +1,6 @@ +with Ada.Directories; with Ada.Strings.Fixed; +with Ada.Text_IO; with Alire.Crates; with Alire.Defaults; @@ -13,6 +15,8 @@ with GNAT.IO; -- To keep preelaborable with Semantic_Versioning.Basic; with Semantic_Versioning.Extended; +with TOML.File_IO; + package body Alire.Releases is package Semver renames Semantic_Versioning; @@ -581,7 +585,8 @@ package body Alire.Releases is (TOML_Adapters.From (TOML_Load.Load_File (File_Name), "Loading release from manifest: " & File_Name), - Source); + Source, + File_Name); exception when E : others => -- As this file is edited manually, it may not load for many reasons @@ -593,8 +598,9 @@ package body Alire.Releases is -- From_TOML -- --------------- - function From_TOML (From : TOML_Adapters.Key_Queue; - Source : Manifest.Sources) + function From_TOML (From : TOML_Adapters.Key_Queue; + Source : Manifest.Sources; + File : Any_Path := "") return Release is begin From.Assert_Key (TOML_Keys.Name, TOML.TOML_String); @@ -602,7 +608,7 @@ package body Alire.Releases is return This : Release := New_Empty_Release (Name => +From.Unwrap.Get (TOML_Keys.Name).As_String) do - Assert (This.From_TOML (From, Source)); + Assert (This.From_TOML (From, Source, File)); end return; end From_TOML; @@ -612,21 +618,36 @@ package body Alire.Releases is function From_TOML (This : in out Release; From : TOML_Adapters.Key_Queue; - Source : Manifest.Sources) + Source : Manifest.Sources; + File : Any_Path := "") return Outcome is + package Dirs renames Ada.Directories; package Labeled renames Alire.Properties.Labeled; begin Trace.Debug ("Loading release " & This.Milestone.Image); -- Origin - This.Origin.From_TOML (From).Assert; + case Source is + when Manifest.Index => + This.Origin.From_TOML (From).Assert; + when Manifest.Local => + This.Origin := + Origins.New_Filesystem + (Dirs.Containing_Directory -- workspace folder + (Dirs.Containing_Directory -- alire folder + (Dirs.Full_Name (File)))); -- absolute path + -- We don't require an origin for a local release, as the release + -- is already in place. + end case; -- Properties TOML_Load.Load_Crate_Section - (Crates.Release_Section, + ((case Source is + when Manifest.Index => Crates.Index_Release, + when Manifest.Local => Crates.Local_Release), From, This.Properties, This.Dependencies, @@ -666,11 +687,35 @@ package body Alire.Releases is Semver.Extended.To_Extended (Semver.Basic.Exactly (R.Version))))); + ------------- + -- To_File -- + ------------- + + procedure To_File (R : Release; + Filename : String; + Format : Manifest.Sources) is + use Ada.Text_IO; + File : File_Type; + begin + Create (File, Out_File, Filename); + TOML.File_IO.Dump_To_File (R.To_TOML (Format), File); + Close (File); + exception + when others => + if Is_Open (File) then + Close (File); + end if; + raise; + end To_File; + ------------- -- To_TOML -- ------------- - overriding function To_TOML (R : Release) return TOML.TOML_Value is + function To_TOML (R : Release; + Format : Manifest.Sources) + return TOML.TOML_Value + is package APL renames Alire.Properties.Labeled; use all type Alire.Properties.Labeled.Cardinalities; use all type Alire.Requisites.Tree; @@ -712,7 +757,12 @@ package body Alire.Releases is end loop; -- Origin - Root.Set (TOML_Keys.Origin, R.Origin.To_TOML); + case Format is + when Manifest.Index => + Root.Set (TOML_Keys.Origin, R.Origin.To_TOML); + when Manifest.Local => + null; + end case; -- Dependencies, wrapped as an array if not R.Dependencies.Is_Empty then diff --git a/src/alire/alire-releases.ads b/src/alire/alire-releases.ads index 1f0846d4..a4e581da 100644 --- a/src/alire/alire-releases.ads +++ b/src/alire/alire-releases.ads @@ -27,10 +27,7 @@ package Alire.Releases is -- subtype Dependency_Vector is Dependencies.Vectors.Vector; - type Release (<>) is - new Interfaces.Tomifiable - and Interfaces.Yamlable - with private; + type Release (<>) is new Interfaces.Yamlable with private; function "<" (L, R : Release) return Boolean; @@ -278,16 +275,25 @@ package Alire.Releases is return Release; function From_TOML (From : TOML_Adapters.Key_Queue; - Source : Manifest.Sources) - return Release; - -- Load a release from a TOML table + Source : Manifest.Sources; + File : Any_Path := "") + return Release + with Pre => Source not in Manifest.Local or else File /= ""; + -- Load a release from a TOML table. We require the manifest file for local + -- manifests to be able to construct a local filesystem origin. - overriding - function To_TOML (R : Release) return TOML.TOML_Value; + function To_TOML (R : Release; + Format : Manifest.Sources) + return TOML.TOML_Value; overriding function To_YAML (R : Release) return String; + procedure To_File (R : Release; + Filename : String; + Format : Manifest.Sources); + -- Directly write the release manifest to a file + function Version_Image (R : Release) return String; private @@ -303,9 +309,8 @@ private -- Properties that R has under platform properties P type Release (Prj_Len, - Notes_Len : Natural) is - new Interfaces.Tomifiable - and Interfaces.Yamlable + Notes_Len : Natural) + is new Interfaces.Yamlable with record Name : Crate_Name (Prj_Len); Alias : UString; -- I finally gave up on constraints @@ -320,9 +325,12 @@ private function From_TOML (This : in out Release; From : TOML_Adapters.Key_Queue; - Source : Manifest.Sources) - return Outcome; - -- Fill in an already existing release + Source : Manifest.Sources; + File : Any_Path := "") + return Outcome + with Pre => Source not in Manifest.Local or else File /= ""; + -- Fill in an already existing release. We require the manifest file + -- location for local releases to be able to construct a local file origin. use all type Conditional.Properties; diff --git a/src/alire/alire-toml_load.adb b/src/alire/alire-toml_load.adb index 905db71d..e1d617a3 100644 --- a/src/alire/alire-toml_load.adb +++ b/src/alire/alire-toml_load.adb @@ -12,16 +12,20 @@ package body Alire.TOML_Load is -- properties, but stored separately as complex types. type Tables is (Available, - Dependencies); + Dependencies, + Origin); Allowed_Tables : constant array (Crates.Sections, Tables) of Boolean := - (Crates.Release_Section => + (Crates.Index_Release => (others => True), + Crates.Local_Release => + (Origin => False, + others => True), Crates.External_Shared_Section => (others => False), Crates.External_Private_Section => - (Available => True, - Dependencies => False)); + (Available => True, + others => False)); ------------------ -- Format_Error -- diff --git a/src/alire/alire-workspace.adb b/src/alire/alire-workspace.adb index 357fed96..3e39cb69 100644 --- a/src/alire/alire-workspace.adb +++ b/src/alire/alire-workspace.adb @@ -5,10 +5,10 @@ with Alire.Dependencies.Containers; with Alire.Dependencies.States; with Alire.Directories; with Alire.Lockfiles; +with Alire.Manifest; with Alire.Origins.Deployers; with Alire.OS_Lib; with Alire.Properties.Actions.Executor; -with Alire.Releases.TOML_IO; with Alire.Roots; with Alire.Solutions.Diffs; with Alire.Workspace; @@ -250,7 +250,7 @@ package body Alire.Workspace is Directories.Backup_If_Existing (Root.Crate_File); - Alire.Releases.TOML_IO.To_File (Release, Root.Crate_File); + Release.To_File (Root.Crate_File, Manifest.Local); end Generate_Manifest; ------------ diff --git a/testsuite/drivers/alr.py b/testsuite/drivers/alr.py index fe3b27d9..0a27c17b 100644 --- a/testsuite/drivers/alr.py +++ b/testsuite/drivers/alr.py @@ -162,3 +162,15 @@ name = '{}' priority = {} url = '{}' """.format(name, priority, os.path.join(working_dir, files_dir))) + + +def init_local_crate(name="xxx", binary=True): + """ + Initialize a local crate and enter its folder for further testing. + + :param str name: Name of the crate + + :param bool binary: Initialize as --bin or --lib + """ + run_alr("init", name, "--bin" if binary else "--lib") + os.chdir(name) diff --git a/testsuite/drivers/helpers.py b/testsuite/drivers/helpers.py index 8b40e3be..0815e642 100644 --- a/testsuite/drivers/helpers.py +++ b/testsuite/drivers/helpers.py @@ -13,7 +13,7 @@ def contents(dir): for name in dirs + files]) -# Return the content of a text file +# Return the content of a text file as a single string with embedded newlines def content_of(filename): out = '' with open(filename, 'r') as f: @@ -22,6 +22,14 @@ def content_of(filename): return out +def lines_of(filename): + """ + Return the contents of a file as an array of lines (with line breaks) + """ + with open(filename, 'r') as f: + return f.readlines() + + # Assert two values are equal or format the differences def compare(found, wanted): assert found == wanted, 'Got: {}\nWanted: {}'.format(found, wanted) diff --git a/testsuite/tests/misc/local-no-origin/test.py b/testsuite/tests/misc/local-no-origin/test.py new file mode 100644 index 00000000..15466804 --- /dev/null +++ b/testsuite/tests/misc/local-no-origin/test.py @@ -0,0 +1,27 @@ +""" +Verify that workspace metadata does not specify an origin +""" + +from drivers.alr import run_alr, init_local_crate +from drivers.helpers import content_of +from glob import glob +from os import chdir +from os.path import join + + +def assert_no_origin(crate): + assert "origin" not in content_of(join("alire", crate + ".toml")), \ + "found unexpected contents in manifest of crate " + crate + + +# case 1, a gotten crate +p = run_alr('get', 'libhello') +chdir(glob("libhello*")[0]) +assert_no_origin("libhello") +chdir("..") + +# case 2, a fresh crate +init_local_crate("xxx") +assert_no_origin("xxx") + +print('SUCCESS') diff --git a/testsuite/tests/misc/local-no-origin/test.yaml b/testsuite/tests/misc/local-no-origin/test.yaml new file mode 100644 index 00000000..872fc127 --- /dev/null +++ b/testsuite/tests/misc/local-no-origin/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + basic_index: {} diff --git a/testsuite/tests/misc/local-reject-origin/test.py b/testsuite/tests/misc/local-reject-origin/test.py new file mode 100644 index 00000000..a5521d01 --- /dev/null +++ b/testsuite/tests/misc/local-reject-origin/test.py @@ -0,0 +1,19 @@ +""" +Verify that an origin definition is rejected in a local manifest +""" + +from drivers.alr import run_alr, init_local_crate +from drivers.asserts import assert_match +from os.path import join + + +init_local_crate("xxx") + +# Add manually the origin, and verify that it cannot be loaded +with open(join("alire", "xxx.toml"), "a") as file: + file.write("\n[origin]\n") + +p = run_alr("show", complain_on_error=False) +assert_match(".*invalid property: origin.*", p.out) + +print('SUCCESS') diff --git a/testsuite/tests/misc/local-reject-origin/test.yaml b/testsuite/tests/misc/local-reject-origin/test.yaml new file mode 100644 index 00000000..32c747b3 --- /dev/null +++ b/testsuite/tests/misc/local-reject-origin/test.yaml @@ -0,0 +1 @@ +driver: python-script diff --git a/testsuite/tests/setenv/linked-paths/my_index/crates/crate_1234/alire/crate_1234.toml b/testsuite/tests/setenv/linked-paths/my_index/crates/crate_1234/alire/crate_1234.toml index 4a89b495..4716db1e 100644 --- a/testsuite/tests/setenv/linked-paths/my_index/crates/crate_1234/alire/crate_1234.toml +++ b/testsuite/tests/setenv/linked-paths/my_index/crates/crate_1234/alire/crate_1234.toml @@ -4,6 +4,3 @@ description = "Shiny new project" maintainers = ["your@email.here"] maintainers-logins = ["github-username"] project-files = ["nested/project.gpr"] - -[origin] -url = "file:.." diff --git a/testsuite/tests/show/inside-vs-outside/test.py b/testsuite/tests/show/inside-vs-outside/test.py index f840a2b5..3c5999e2 100644 --- a/testsuite/tests/show/inside-vs-outside/test.py +++ b/testsuite/tests/show/inside-vs-outside/test.py @@ -9,6 +9,21 @@ from glob import glob from drivers.alr import run_alr from drivers.asserts import assert_eq + +def detach_origin(lines): + # Return lines without "Origin:", and the origin separately + # Initially the lines is simply the output, so we split: + lines = lines.split('\n') + newlines = [] + origin = "" + for line in lines: + if line.startswith("Origin:"): + origin = line + else: + newlines.append(line) + return '\n'.join(newlines), origin + + # Outside run run1 = run_alr('show', 'libhello') @@ -18,6 +33,14 @@ os.chdir(glob('libhello*')[0]) # Inside run run2 = run_alr('show') -assert_eq(run1.out, run2.out) +# The origin will change from inside the index to the deployment folder, +# so we have to compare that separately +out1, origin1 = detach_origin(run1.out) +out2, origin2 = detach_origin(run2.out) + +assert_eq(out1, out2) + +assert origin1.endswith("libhello_1.0.0") # Original source folder +assert origin2.endswith("libhello_1.0.0_filesystem") # Created by `alr get` print('SUCCESS') -- 2.39.5