From 878c42131c1325b22cc0a1905e5d47b9998f5954 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Mon, 28 Jun 2021 16:07:05 +0200 Subject: [PATCH] Implement `--branch` in `alr with`, `alr pin` (#760) * Completed Roots.Editable commands for removal * Completed Alr.Commands.Pin * Reinstated all `alr pin` functionality * Reinstated remaining disabled pin tests * Reinstated remaining disabled tests * Testsuite: fix platform-independent path on Windows * Implement pin commands in alr driver * Testsuite: Implement with command in alr driver * Testsuite: equivalence between manual and cmd-line * Fix git folder deletion on Windows --- src/alire/alire-directories.adb | 10 ++ src/alire/alire-directories.ads | 3 + src/alire/alire-user_pins.adb | 2 +- src/alr/alr-commands-pin.adb | 18 ++- src/alr/alr-commands-pin.ads | 3 +- src/alr/alr-commands-withing.adb | 27 ++-- src/alr/alr-commands-withing.ads | 3 +- testsuite/drivers/alr.py | 144 ++++++++++++++++++---- testsuite/tests/pin/branch/test.py | 2 +- testsuite/tests/pin/change-type/test.py | 2 +- testsuite/tests/pin/equivalent/test.py | 66 ++++++++++ testsuite/tests/pin/equivalent/test.yaml | 3 + testsuite/tests/with/equivalent/test.py | 70 +++++++++++ testsuite/tests/with/equivalent/test.yaml | 3 + 14 files changed, 314 insertions(+), 42 deletions(-) create mode 100644 testsuite/tests/pin/equivalent/test.py create mode 100644 testsuite/tests/pin/equivalent/test.yaml create mode 100644 testsuite/tests/with/equivalent/test.py create mode 100644 testsuite/tests/with/equivalent/test.yaml diff --git a/src/alire/alire-directories.adb b/src/alire/alire-directories.adb index e0da9669..d9332a28 100644 --- a/src/alire/alire-directories.adb +++ b/src/alire/alire-directories.adb @@ -117,6 +117,16 @@ package body Alire.Directories is End_Search (Search); end Copy; + ----------------- + -- Delete_Tree -- + ----------------- + + procedure Delete_Tree (Path : Any_Path) is + begin + Ensure_Deletable (Path); + Ada.Directories.Delete_Tree (Path); + end Delete_Tree; + ---------------------- -- Detect_Root_Path -- ---------------------- diff --git a/src/alire/alire-directories.ads b/src/alire/alire-directories.ads index 285de4b7..b98313bb 100644 --- a/src/alire/alire-directories.ads +++ b/src/alire/alire-directories.ads @@ -32,6 +32,9 @@ package Alire.Directories is return String; -- Return either the valid enclosing root folder, or "" + procedure Delete_Tree (Path : Any_Path); + -- Equivalent to Ensure_Deletable + Ada.Directories.Delete_Tree + procedure Ensure_Deletable (Path : Any_Path); -- In Windows, git checkouts are created with read-only file that do not -- sit well with Ada.Directories.Delete_Tree. diff --git a/src/alire/alire-user_pins.adb b/src/alire/alire-user_pins.adb index c0f42e2d..6fb77635 100644 --- a/src/alire/alire-user_pins.adb +++ b/src/alire/alire-user_pins.adb @@ -201,7 +201,7 @@ package body Alire.User_Pins is then Put_Info ("Switching pin " & TTY.Name (Crate) & " to origin at " & TTY.URL (+This.URL)); - Ada.Directories.Delete_Tree (Destination); + Directories.Delete_Tree (Destination); Checkout; -- Pending branch tracking implementation return; end if; diff --git a/src/alr/alr-commands-pin.adb b/src/alr/alr-commands-pin.adb index 1ba2b362..3ceef83e 100644 --- a/src/alr/alr-commands-pin.adb +++ b/src/alr/alr-commands-pin.adb @@ -122,6 +122,9 @@ package body Alr.Commands.Pin is then Reportaise_Wrong_Arguments ("--use must be used alone with a crate name"); + elsif Cmd.Commit.all /= "" and then Cmd.Branch.all /= "" then + Reportaise_Wrong_Arguments + ("Cannot specify both a branch and a commit simultaneously"); end if; Cmd.Requires_Valid_Session; @@ -158,7 +161,7 @@ package body Alr.Commands.Pin is elsif Cmd.URL.all /= "" then - if Cmd.Commit.all /= "" + if Cmd.Commit.all /= "" or else Cmd.Branch.all /= "" or else Alire.URI.Is_HTTP_Or_Git (Cmd.URL.all) then @@ -168,7 +171,7 @@ package body Alr.Commands.Pin is (Crate => Optional_Crate, Origin => Cmd.URL.all, Commit => Cmd.Commit.all, - Branch => ""); -- TODO: allow passing --branch + Branch => Cmd.Branch.all); else @@ -237,7 +240,9 @@ package body Alr.Commands.Pin is & " instead of looking for indexed releases." & " An optional reference can be specified with --commit;" & " the pin will be frozen at the commit currently matching" - & " the reference.") + & " the reference. Alternatively, a branch to track can be" + & " specified with --branch. Use `alr update` to refresh the" + & " tracking pin contents.") ); -------------------- @@ -261,6 +266,13 @@ package body Alr.Commands.Pin is Long_Switch => "--unpin", Help => "Unpin a release"); + Define_Switch + (Config => Config, + Output => Cmd.Branch'Access, + Long_Switch => "--branch=", + Argument => "NAME", + Help => "Branch to be tracked in repository"); + Define_Switch (Config => Config, Output => Cmd.Commit'Access, diff --git a/src/alr/alr-commands-pin.ads b/src/alr/alr-commands-pin.ads index ffb27997..ae2d9cb4 100644 --- a/src/alr/alr-commands-pin.ads +++ b/src/alr/alr-commands-pin.ads @@ -23,12 +23,13 @@ package Alr.Commands.Pin is overriding function Usage_Custom_Parameters (Cmd : Command) return String is ("[[crate[=]]" - & " | crate --use= [--commit=REF]" + & " | crate --use= [--commit=REF] [--branch=NAME]" & " | --all]"); private type Command is new Commands.Command with record + Branch : aliased GNAT.Strings.String_Access; Commit : aliased GNAT.Strings.String_Access; Pin_All : aliased Boolean; Unpin : aliased Boolean; diff --git a/src/alr/alr-commands-withing.adb b/src/alr/alr-commands-withing.adb index 2e6c925d..95b395b1 100644 --- a/src/alr/alr-commands-withing.adb +++ b/src/alr/alr-commands-withing.adb @@ -185,7 +185,7 @@ package body Alr.Commands.Withing is -- Now, add the pin to the path/remote - if Cmd.Commit.all /= "" + if Cmd.Commit.all /= "" or else Cmd.Branch.all /= "" or else Alire.URI.Is_HTTP_Or_Git (Cmd.URL.all) then @@ -195,7 +195,7 @@ package body Alr.Commands.Withing is (Crate => Crate, Origin => Cmd.URL.all, Commit => Cmd.Commit.all, - Branch => ""); -- TODO: PARSE BRANCH + Branch => Cmd.Branch.all); else @@ -241,6 +241,11 @@ package body Alr.Commands.Withing is Check (Cmd.Tree); Check (Cmd.Versions); + if Cmd.Commit.all /= "" and then Cmd.Branch.all /= "" then + Reportaise_Wrong_Arguments + ("Cannot specify both a branch and a commit simultaneously"); + end if; + -- No parameters: give requested info and return. There is still the -- possibility of a `with --use` that is processed later. @@ -337,7 +342,9 @@ package body Alr.Commands.Withing is & " version by the sources found at the given target." & " An optional reference can be specified with --commit;" & " the pin will be frozen at the commit currently matching" - & " the reference.") + & " the reference. Alternatively, a branch to track can be" + & " specified with --branch. Use `alr update` to refresh the" + & " tracking pin contents.") .New_Line .Append ("* Adding dependencies from a GPR file:") .Append ("The project file given with --from will be scanned looking" @@ -351,13 +358,6 @@ package body Alr.Commands.Withing is .New_Line .Append ("with ""libhello""; -- alr with libhello") .New_Line - .Append ("* Caveat:") - .Append ("Since alr does not modify user files, any dependencies" - & " managed through this command only directly affect" - & " the metadata files of alr itself. In order to use the" - & " dependencies in Ada code, the user *must* add the needed" - & " 'with'ed project files in their own GPR files.") - .New_Line .Append (Crate_Version_Sets)); -------------------- @@ -385,6 +385,13 @@ package body Alr.Commands.Withing is "", "--graph", "Show ASCII graph of dependencies"); + Define_Switch + (Config => Config, + Output => Cmd.Branch'Access, + Long_Switch => "--branch=", + Argument => "NAME", + Help => "Branch to track in repository"); + Define_Switch (Config => Config, Output => Cmd.Commit'Access, diff --git a/src/alr/alr-commands-withing.ads b/src/alr/alr-commands-withing.ads index 11217f69..65aa8b49 100644 --- a/src/alr/alr-commands-withing.ads +++ b/src/alr/alr-commands-withing.ads @@ -20,12 +20,13 @@ package Alr.Commands.Withing is overriding function Usage_Custom_Parameters (Cmd : Command) return String is ("[{ [--del] [versions]..." & " | --from ..." - & " | [versions] --use [--commit REF} ]" + & " | [versions] --use [--commit REF] [--branch NAME]} ]" & " | --solve | --tree | --versions"); private type Command is new Commands.Command with record + Branch : aliased GNAT.Strings.String_Access; Commit : aliased GNAT.Strings.String_Access; Del : aliased Boolean := False; From : aliased Boolean := False; diff --git a/testsuite/drivers/alr.py b/testsuite/drivers/alr.py index 1ead09c0..028e2194 100644 --- a/testsuite/drivers/alr.py +++ b/testsuite/drivers/alr.py @@ -220,43 +220,56 @@ def alr_touch_manifest(path="."): os.utime(os.path.join(path, "alire.lock"), (0, 0)) -def alr_unpin(crate, manual=True, fail_if_missing=True, update=True): +def delete_array_entry_from_manifest(array, crate, + fail_if_missing=True, update=True): """ - Unpin a crate, if pinned, or no-op otherwise. Will edit the manifest or use - the command-line, according to manual. Must be run in a crate root. - If update, run `alr pin` to force computation of new solution + Manual deletion of dependencies/pins. As we emit the additions too, this is + simpler than the actual code in alr. """ - - if manual: - # Locate and remove the lines with the pin - with open("alire.toml", "rt") as manifest: + with open(alr_manifest(), "rt") as manifest: found = False lines = manifest.readlines() orig = lines for i in range(1, len(lines)): if lines[i].startswith(f"{crate} =") \ - and lines[i-1] == "[[pins]]\n": + and lines[i-1] == f"[[{array}]]\n": lines.pop(i) lines.pop(i-1) found = True break - # Write the new manifest - if found: - with open("alire.toml", "wt") as manifest: - manifest.writelines(lines) + # Write the new manifest + if found: + with open(alr_manifest(), "wt") as manifest: + manifest.writelines(lines) + if update: + run_alr("pin") # Ensure changes don't affect next command output + elif fail_if_missing: + raise RuntimeError + (f"Could not remove crate {crate} in lines:\n" + str(orig)) - # Make the lockfile "older" (otherwise timestamp is identical) - os.utime("alire.lock", (0, 0)) + # Make the lockfile "older" (otherwise timestamp is identical) + os.utime(alr_lockfile(), (0, 0)) - if update: - run_alr("pin") # Ensure changes don't affect next command output - elif fail_if_missing: - raise RuntimeError - (f"Could not unpin crate {crate} in lines:\n" + str(orig)) + +def alr_unpin(crate, manual=True, fail_if_missing=True, update=True): + """ + Unpin a crate, if pinned, or no-op otherwise. Will edit the manifest or use + the command-line, according to manual. Must be run in a crate root. + If update, run `alr pin` to force computation of new solution + """ + + if manual: + # Locate and remove the lines with the pin + delete_array_entry_from_manifest("pins", crate, + fail_if_missing, update) else: - raise NotImplementedError("Unimplemented") + if not update: + raise RuntimeError("Update cannot be disabled when using the" + " command-line interface") + + run_alr("pin", "--unpin", crate) def alr_pin(crate, version="", path="", url="", commit="", branch="", @@ -267,6 +280,9 @@ def alr_pin(crate, version="", path="", url="", commit="", branch="", When update, run `alr pin` so the new solution is computed. """ + if commit != "" and branch != "": + raise RuntimeError("Do not specify both commit and branch") + if manual: alr_unpin(crate, fail_if_missing=False) # Just in case @@ -283,14 +299,94 @@ def alr_pin(crate, version="", path="", url="", commit="", branch="", else: raise ValueError("Specify either version, path or url") - with open("alire.toml", "at") as manifest: + with open(alr_manifest(), "at") as manifest: manifest.writelines(["\n[[pins]]\n", pin_line + "\n"]) # Make the lockfile "older" (otherwise timestamp is identical) - os.utime("alire.lock", (0, 0)) + os.utime(alr_lockfile(), (0, 0)) if update: run_alr("pin") # so the changes in the manifest are applied else: - raise NotImplementedError("Unimplemented") + if not update: + raise RuntimeError("Update cannot be disabled when using the" + " command-line interface") + + args = [] + if version != "": + args += [f"{crate}={version}"] + else: + args += [crate] + + if path != "": + args += ["--use", f"{path}"] + elif url != "": + args += ["--use", f"{url}"] + + if commit != "": + args += ["--commit", f"{commit}"] + elif branch != "": + args += ["--branch", f"{branch}"] + + run_alr("pin", *args) + + +def alr_with(dep="", path="", url="", commit="", branch="", + delete=False, manual=True, update=True, force=False): + """ + Add/remove dependencies either through command-line or manifest edition + """ + if commit != "" and branch != "": + raise RuntimeError("Do not specify both commit and branch") + if path != "" and url != "": + raise RuntimeError("Do not specify both path and url") + + separators = "=^~<>*" + + # Fix the dependency if no version subset is in dep + if not any([separator in dep for separator in separators]): + dep += "*" + + # Find the separator position + pos = max([dep.find(separator) for separator in separators]) + + if manual: + if delete: + delete_array_entry_from_manifest("depends-on", dep, update=update) + else: + with open(alr_manifest(), "at") as manifest: + lines = ["\n[[depends-on]]\n", + f'{dep[:pos]} = "{dep[pos:]}"\n'] + manifest.writelines(lines) + + if path != "" or url != "": + alr_pin(crate=f'{dep[:pos]}', path=path, url=url, + commit=commit, branch=branch, manual=manual, + update=False) + + # Make the lockfile "older" (otherwise timestamp is identical) + os.utime(alr_lockfile(), (0, 0)) + + if update: + run_alr("with", force=force) + + else: + if delete: + run_alr("with", "--del", dep) + else: + args = ["with"] + if dep != "": + args += [dep] + + if path != "": + args += ["--use", f"{path}"] + elif url != "": + args += ["--use", f"{url}"] + + if commit != "": + args += ["--commit", f"{commit}"] + elif branch != "": + args += ["--branch", f"{branch}"] + + run_alr(*args, force=force) diff --git a/testsuite/tests/pin/branch/test.py b/testsuite/tests/pin/branch/test.py index 061db51b..53d2806c 100644 --- a/testsuite/tests/pin/branch/test.py +++ b/testsuite/tests/pin/branch/test.py @@ -13,7 +13,7 @@ import os import shutil import subprocess -# "remote" is going to be the remote crate +# "remote" is going to be the remote crate init_local_crate(name="remote", enter=False) url = os.path.join(os.getcwd(), "remote") diff --git a/testsuite/tests/pin/change-type/test.py b/testsuite/tests/pin/change-type/test.py index b7cafe20..9712850f 100644 --- a/testsuite/tests/pin/change-type/test.py +++ b/testsuite/tests/pin/change-type/test.py @@ -34,7 +34,7 @@ alr_pin('libhello', path='../crates/libhello_1.0.0') # Check that it shows as such in the solution p = run_alr('show', '--solve') assert_match('.*Dependencies \(external\):.*' - 'libhello\^1 \(direct,linked' + 'libhello.*\^1.*\(direct,linked' ',path=../crates/libhello_1.0.0\).*', p.out, flags=re.S) diff --git a/testsuite/tests/pin/equivalent/test.py b/testsuite/tests/pin/equivalent/test.py new file mode 100644 index 00000000..b6695240 --- /dev/null +++ b/testsuite/tests/pin/equivalent/test.py @@ -0,0 +1,66 @@ +""" +Verify that using manual edition and `alr pin` result in equivalent outputs +""" + +from drivers.alr import run_alr, alr_pin, init_local_crate +from drivers.asserts import assert_eq, assert_match +from drivers.helpers import init_git_repo, git_branch + +import os +import shutil + + +def check_equivalent(crate, path="", url="", commit="", branch=""): + """ + Run manual and auto and compare outputs + """ + manual = [False, True] + p = [None, None] + + for i in [0, 1]: + init_local_crate() + + # run command + alr_pin(crate=crate, path=path, url=url, + commit=commit, branch=branch, + manual=manual[i]) + + # get pins output + p[i] = run_alr("pin").out + + if i == 1: + assert_eq(p[0], p[1]) + + # Cleanup + os.chdir("..") + shutil.rmtree("xxx") + + +# Local pinnable crate +init_local_crate("yyy", enter=False) +yyy_path = "../yyy" + +# Local pinnable raw project +os.mkdir("zzz") +zzz_path = "../zzz" + +# Simple pin, no restrictions +check_equivalent("yyy", path=yyy_path) +check_equivalent("zzz", path=zzz_path) + +# Prepare repository +head = init_git_repo("yyy") +branch = git_branch("yyy") +os.rename("yyy", "yyy.git") # to be recognizable as a git url +url = "../yyy.git" + +# Simple git remote, explicit crate +check_equivalent(crate="yyy", url=url) + +# Explicit commit +check_equivalent(crate="yyy", url=url, commit=head) + +# Explicit branch +check_equivalent(crate="yyy", url=url, branch=branch) + +print('SUCCESS') diff --git a/testsuite/tests/pin/equivalent/test.yaml b/testsuite/tests/pin/equivalent/test.yaml new file mode 100644 index 00000000..872fc127 --- /dev/null +++ b/testsuite/tests/pin/equivalent/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + basic_index: {} diff --git a/testsuite/tests/with/equivalent/test.py b/testsuite/tests/with/equivalent/test.py new file mode 100644 index 00000000..28cf3f74 --- /dev/null +++ b/testsuite/tests/with/equivalent/test.py @@ -0,0 +1,70 @@ +""" +Verify that using manual edition and `alr with` result in equivalent manifests +""" + +from drivers.alr import run_alr, alr_with, init_local_crate +from drivers.asserts import assert_eq, assert_match +from drivers.helpers import init_git_repo, git_branch + +import os +import shutil + + +def check_equivalent(dep="", path="", url="", commit="", branch=""): + """ + Run manual and auto and compare outputs + """ + manual = [False, True] + p = [None, None] + + for i in [0, 1]: + init_local_crate() + + # run command + alr_with(dep=dep, path=path, url=url, + commit=commit, branch=branch, force=True, + manual=manual[i]) + + # get output of solution + p[i] = run_alr("with", "--solve").out + + if i == 1: + assert_eq(p[0], p[1]) + + # Cleanup + os.chdir("..") + shutil.rmtree("xxx") + + +# Simple with without subset cannot be tested as `alr with` will narrow down +# the dependency causing a discrepancy + +# Existing crate with subset +check_equivalent("libhello^1") + +# Non-existent version +check_equivalent("libhello^777") + +# Non-existent crate +check_equivalent("unobtanium") + +# Pinned folder +init_local_crate("yyy", enter=False) +check_equivalent("yyy~0", path="../yyy") + +# Prepare repository +head = init_git_repo("yyy") +branch = git_branch("yyy") +os.rename("yyy", "yyy.git") # to be recognizable as a git url +url = "../yyy.git" + +# Simple git remote, explicit crate & version +check_equivalent(dep="yyy~0", url=url) + +# Explicit commit +check_equivalent(dep="yyy~0", url=url, commit=head) + +# Explicit branch +check_equivalent(dep="yyy~0", url=url, branch=branch) + +print('SUCCESS') diff --git a/testsuite/tests/with/equivalent/test.yaml b/testsuite/tests/with/equivalent/test.yaml new file mode 100644 index 00000000..872fc127 --- /dev/null +++ b/testsuite/tests/with/equivalent/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + basic_index: {} -- 2.39.5