From d1c1fbfcd3c67bcd6e1bffacc5c47e06d06224ae Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Mon, 6 Nov 2023 11:14:55 +0100 Subject: [PATCH] Fix list of default licenses (#1490) * Remove duplication in error message output * Early detect bad license among choices * Fix bad license * Fix error deduplication * New test for default licenses --- .github/workflows/ci-toolchain.yml | 2 +- src/alire/alire-errors.adb | 19 ++++--- src/alire/alire-properties-licenses.adb | 13 ++--- src/alr/alr-commands-init.adb | 13 +++-- testsuite/Dockerfile | 5 +- testsuite/drivers/alr.py | 49 ++++++++++++++++++- testsuite/requirements.txt | 1 + testsuite/tests/init/default-licenses/test.py | 33 +++++++++++++ .../tests/init/default-licenses/test.yaml | 2 + 9 files changed, 114 insertions(+), 23 deletions(-) create mode 100644 testsuite/tests/init/default-licenses/test.py create mode 100644 testsuite/tests/init/default-licenses/test.yaml diff --git a/.github/workflows/ci-toolchain.yml b/.github/workflows/ci-toolchain.yml index 41d7442f2..d2bc82809 100644 --- a/.github/workflows/ci-toolchain.yml +++ b/.github/workflows/ci-toolchain.yml @@ -72,7 +72,7 @@ jobs: python-version: '3.x' - name: Install e3 - run: pip install --upgrade e3-testsuite + run: pip install --upgrade -r testsuite/requirements.txt - name: Run testsuite run: cd testsuite; ./run.py -E diff --git a/src/alire/alire-errors.adb b/src/alire/alire-errors.adb index 34d5f2448..7b27a2b06 100644 --- a/src/alire/alire-errors.adb +++ b/src/alire/alire-errors.adb @@ -1,4 +1,3 @@ -with Ada.Containers.Indefinite_Doubly_Linked_Lists; with Ada.Containers.Indefinite_Ordered_Maps; package body Alire.Errors is @@ -166,10 +165,7 @@ package body Alire.Errors is -- ERROR STACKING -- -------------------- - package String_Lists is - new Ada.Containers.Indefinite_Doubly_Linked_Lists (String); - - Error_Stack : String_Lists.List; + Error_Stack : AAA.Strings.Vector; ---------- -- Open -- @@ -177,7 +173,7 @@ package body Alire.Errors is function Open (Text : String) return Scope is begin - Error_Stack.Append (Text); + Open (Text); return (Ada.Finalization.Limited_Controlled with null record); end Open; @@ -219,8 +215,15 @@ package body Alire.Errors is Msg : UString; use UStrings; begin - for Item of Error_Stack loop - Append (Msg, Item & ASCII.LF); + -- Remove duplicates that may have creeped in when generating the final + -- stack: + + for I in Error_Stack.First_Index .. Error_Stack.Last_Index loop + if I = Error_Stack.First_Index + or else Error_Stack (I) /= Error_Stack (I - 1) + then + Append (Msg, Error_Stack (I) & ASCII.LF); + end if; end loop; return +Msg & Text; diff --git a/src/alire/alire-properties-licenses.adb b/src/alire/alire-properties-licenses.adb index 5a1ca5132..138333782 100644 --- a/src/alire/alire-properties-licenses.adb +++ b/src/alire/alire-properties-licenses.adb @@ -66,10 +66,11 @@ package body Alire.Properties.Licenses is if not SPDX.Valid (SPDX_Exp) then if Legacy = Licensing.Unknown then raise Checked_Error - with Errors.Set - ("Invalid license expression '" & From & - "': " & SPDX.Error (SPDX_Exp) & - " . SPDX expression expected (https://spdx.org/licenses/)"); + with Errors.New_Wrapper + ("Invalid license expression '" & From) + .Wrap (SPDX.Error (SPDX_Exp)) + .Wrap ("SPDX expression expected (https://spdx.org/licenses/)") + .Set; else Trace.Warning ("Deprecated license identifier '" & From & "'. Please replace with an SPDX expression " & @@ -121,10 +122,6 @@ package body Alire.Properties.Licenses is end if; return Props; - exception - when E : Checked_Error => -- May happen on unknown non-custom license. - From.Checked_Error (Errors.Get (E)); - -- Re-raise with full context of From. end From_TOML; end Alire.Properties.Licenses; diff --git a/src/alr/alr-commands-init.adb b/src/alr/alr-commands-init.adb index f7e083178..ab770466e 100644 --- a/src/alr/alr-commands-init.adb +++ b/src/alr/alr-commands-init.adb @@ -433,11 +433,12 @@ package body Alr.Commands.Init is .Append ("Apache-2.0") .Append ("BSD-3-Clause") .Append ("LGPL-3.0-or-later") - .Append ("GPL-3.0-or-later WITH GPL-3.0-with-GCC-exception") + .Append ("GPL-3.0-or-later WITH GCC-exception-3.1") .Append ("GPL-3.0-or-later") .Append (License_Other); - Answer : Natural; + Answer : Natural := 0; + function Chosen return String is (License_Vect (Answer)); begin Answer := CLIC.User_Input.Query_Multi (Question => "Select a software " & Emph ("license") & @@ -456,7 +457,11 @@ package body Alr.Commands.Init is Default => "", Validation => License_Validation'Access)); else - Info.Licenses := To_Unbounded_String (License_Vect (Answer)); + if not License_Validation (Chosen) then + raise Program_Error with + "Invalid license among choices: " & Chosen; + end if; + Info.Licenses := To_Unbounded_String (Chosen); end if; end Query_License; @@ -598,7 +603,7 @@ package body Alr.Commands.Init is Info.Website := To_Unbounded_String (CLIC.User_Input.Query_String - (Question => "Enter a opional " & Emph ("Website URL") & + (Question => "Enter an optional " & Emph ("Website URL") & " for the crate:", Default => "", Validation => null)); diff --git a/testsuite/Dockerfile b/testsuite/Dockerfile index 6eaae2bcc..3a93b9b4f 100644 --- a/testsuite/Dockerfile +++ b/testsuite/Dockerfile @@ -15,7 +15,10 @@ RUN apt-get update && apt-get install -y \ sudo \ util-linux # for `unshare` -RUN pip3 install e3-testsuite +# Use parent testsuite python packages here too, as they are potential imports +# of the drivers and helpers that may be needed in the wrapped test. +COPY ./testsuite/requirements.txt /testsuite/requirements.txt +RUN pip3 install -r /testsuite/requirements.txt WORKDIR /testsuite USER user diff --git a/testsuite/drivers/alr.py b/testsuite/drivers/alr.py index 6ddc8dee0..8ae2fe38e 100644 --- a/testsuite/drivers/alr.py +++ b/testsuite/drivers/alr.py @@ -4,12 +4,15 @@ import os import os.path +import platform +import pexpect import re -from shutil import copytree +import sys from e3.fs import mkdir from e3.os.process import Run, quote_arg from e3.testsuite.driver.classic import ProcessResult +from shutil import copytree TESTSUITE_ROOT = os.path.dirname(os.path.dirname( os.path.abspath(__file__))) @@ -127,6 +130,50 @@ def run_alr(*args, **kwargs): return ProcessResult(p.status, p.out.replace('\r\n', '\n')) +def run_alr_interactive(args: [str], output: [str], input: [str], timeout=5) -> str: + """ + NON-WINDOWS-ONLY + Run "alr" with the given arguments, feeding it the given input. No other + arguments like -q or -d are added (except --no-color). + + :param args: List of arguments to pass to "alr". + :param output: List of strings expected to be output by the subprocess. + :param input: List of strings to feed to the subprocess's standard input. + :param timeout: Timeout in seconds for the subprocess to complete. + """ + # Check whether on Windows to fail early (revisit if pexpect is updated?) + if platform.system() == "Windows": + print('SKIP: pexpect unavailable on Windows') + sys.exit(0) + + # Run interactively using pexpect (run with input fails as it is not + # detected as tty and input is closed prematurely) + args.insert(0, "--no-color") + child = pexpect.spawn('alr', args=args, timeout=timeout) + + try: + # Alternate between expected output and given input + for out, inp in zip(output, input): + child.expect(out) + child.sendline(inp) + + # Wait for the process to finish + child.expect(pexpect.EOF) # Match all output before ending + child.wait() + child.close() + except pexpect.exceptions.TIMEOUT: + raise RuntimeError(f"pexpect timeout with alr output:\n" + f"{child.before.decode('utf-8')}") + + # Assert proper output code + assert child.exitstatus == 0, \ + f"Unexpected exit status: {child.exitstatus}\n" + \ + f"Output: {child.before.decode('utf-8')}" + + # Return command output with CRLF replaced by LF (as does run_alr) + return child.before.decode('utf-8').replace('\r\n', '\n') + + def fixtures_path(*args): """ Return a path under the testsuite `fixtures` directory. diff --git a/testsuite/requirements.txt b/testsuite/requirements.txt index b0fd578d0..dcaf78ee3 100644 --- a/testsuite/requirements.txt +++ b/testsuite/requirements.txt @@ -1,2 +1,3 @@ docker e3-testsuite +pexpect diff --git a/testsuite/tests/init/default-licenses/test.py b/testsuite/tests/init/default-licenses/test.py new file mode 100644 index 000000000..db8c3892c --- /dev/null +++ b/testsuite/tests/init/default-licenses/test.py @@ -0,0 +1,33 @@ +""" +Check that offered default licenses are all valid +""" + +import os +import shutil + +from drivers.alr import run_alr, run_alr_interactive + +# iterate over values 1..8 +for i in range(1, 9): + + # Run interactively + run_alr_interactive(['init', '--bin', 'xxx'], + output=['> ' for _ in range(7)], + input=['', # Description + '', # Full user name + '', # Github login + '', # Email + f'{i}', # License + '', # Tags + ''], # Website + timeout=3) + + # Check that it can be shown, which will load the manifest + os.chdir("xxx") + p = run_alr("show") + + # Prepare for next iteration + os.chdir("..") + shutil.rmtree("xxx") + +print('SUCCESS') diff --git a/testsuite/tests/init/default-licenses/test.yaml b/testsuite/tests/init/default-licenses/test.yaml new file mode 100644 index 000000000..187c05aa3 --- /dev/null +++ b/testsuite/tests/init/default-licenses/test.yaml @@ -0,0 +1,2 @@ +driver: python-script +indexes: {}