From 4b0ee867b51130fa52ebdc05aac733fa4e6d6c0d Mon Sep 17 00:00:00 2001 From: Hugo Guerrier Date: Tue, 29 Oct 2024 15:24:48 +0100 Subject: [PATCH] Add a depreciation message for '-rules' only shown in TTYs Do this to avoid corruption of report analysis when capturing the GNATcheck output. --- lkql_checker/src/gnatcheck-options.ads | 6 +- lkql_checker/src/gnatcheck-output.adb | 13 +++++ lkql_checker/src/gnatcheck-output.ads | 4 ++ lkql_checker/src/gnatcheck-projects.adb | 18 +++--- testsuite/drivers/base_driver.py | 58 +++++++++++++++++++ testsuite/drivers/gnatcheck_driver.py | 22 ++++--- .../valid_rule_config/test.out | 1 - .../rules_depreciation_message/main.adb | 5 ++ .../rules_depreciation_message/test.out | 3 + .../rules_depreciation_message/test.yaml | 11 ++++ .../same_name_instances/test.out | 1 - 11 files changed, 120 insertions(+), 22 deletions(-) create mode 100644 testsuite/tests/gnatcheck/rules_depreciation_message/main.adb create mode 100644 testsuite/tests/gnatcheck/rules_depreciation_message/test.out create mode 100644 testsuite/tests/gnatcheck/rules_depreciation_message/test.yaml diff --git a/lkql_checker/src/gnatcheck-options.ads b/lkql_checker/src/gnatcheck-options.ads index 1fd6d8fbd..50d3a100f 100644 --- a/lkql_checker/src/gnatcheck-options.ads +++ b/lkql_checker/src/gnatcheck-options.ads @@ -193,9 +193,9 @@ package Gnatcheck.Options is -- Whether the help message about the new instance system has already been -- emitted. This message should be removed in 26.0. - Mixed_Style_Warning_Emitted : Boolean := False; - -- Whether the message about usage of and new and legacy rule options for - -- the same GNATcheck run. + Rules_Depreciation_Emitted : Boolean := False; + -- Whether the message about the ``-rules`` section depreciation has been + -- emitted in the TTY. -------------------------------------- -- Controlling the gnatcheck report -- diff --git a/lkql_checker/src/gnatcheck-output.adb b/lkql_checker/src/gnatcheck-output.adb index 722637745..ca904cf05 100644 --- a/lkql_checker/src/gnatcheck-output.adb +++ b/lkql_checker/src/gnatcheck-output.adb @@ -16,6 +16,8 @@ with GNAT.Traceback.Symbolic; with Gnatcheck.Options; use Gnatcheck.Options; with Gnatcheck.String_Utilities; use Gnatcheck.String_Utilities; +with Interfaces.C_Streams; use Interfaces.C_Streams; + package body Gnatcheck.Output is Report_File_Name : String_Access; @@ -121,6 +123,17 @@ package body Gnatcheck.Output is end if; end Error_No_Tool_Name; + ------------------ + -- Error_In_Tty -- + ------------------ + + procedure Error_In_Tty (Message : String) is + begin + if isatty (fileno (stderr)) /= 0 then + Put_Line (Standard_Error, Executable & ": " & Message); + end if; + end Error_In_Tty; + ----------------------- -- Get_Indent_String -- ----------------------- diff --git a/lkql_checker/src/gnatcheck-output.ads b/lkql_checker/src/gnatcheck-output.ads index 9526ec60d..a874dba37 100644 --- a/lkql_checker/src/gnatcheck-output.ads +++ b/lkql_checker/src/gnatcheck-output.ads @@ -43,6 +43,10 @@ package Gnatcheck.Output is procedure Error_No_Tool_Name (Message : String); -- Sends into Stderr the error message with no tool name prefix + procedure Error_In_Tty (Message : String); + -- Same as ``Error`` but send the message only if Stderr is a TTY. Also, + -- ``Message`` is not added to the stderr log file. + procedure SLOC_Error (Message : String; SLOC : String); diff --git a/lkql_checker/src/gnatcheck-projects.adb b/lkql_checker/src/gnatcheck-projects.adb index f8d7af142..caa3d24fa 100644 --- a/lkql_checker/src/gnatcheck-projects.adb +++ b/lkql_checker/src/gnatcheck-projects.adb @@ -1109,16 +1109,14 @@ package body Gnatcheck.Projects is Add_Rule_Option (Full_Switch (Parser => Parser)); Individual_Rules_Set := True; end case; - if not Mixed_Style_Warning_Emitted then - if Arg.Rules.Get'Length > 0 - or else Arg.Rule_File.Get /= Null_Unbounded_String - then - Warning - ("The '-rules' section usage is discouraged from now, " & - "you should only use the '--rules' and '--rule-file' " & - "command-line options"); - end if; - Mixed_Style_Warning_Emitted := True; + if not Rules_Depreciation_Emitted then + Error_In_Tty + ("The '-rules' section is now deprecated. You should only " & + "use the '--rules' and '--rule-file' command-line options."); + Error_In_Tty + ("You can use the '--emit-lkql-rule-file' flag to " & + "automatically translate your rule configuration to the " & + "new LKQL format."); end if; end loop; end Process_Sections; diff --git a/testsuite/drivers/base_driver.py b/testsuite/drivers/base_driver.py index 93a0ec760..6005d6519 100644 --- a/testsuite/drivers/base_driver.py +++ b/testsuite/drivers/base_driver.py @@ -1,7 +1,11 @@ +import errno import glob import os import os.path as P +import pty import re +import select +import subprocess import sys from typing import TextIO @@ -271,6 +275,60 @@ def run(*prefix): else: raise TestAbortWithError(f"invalid perf mode: {mode}") + def run_in_tty(self, args: list[str], **kwargs) -> tuple[str, int]: + """ + Run a process in a pseudo-TTY using the ``pty`` module. Returns a + tuple containing the process output and its status code. + """ + # Ensure the current system is not windows, according to the + # documentation, the ``pty`` module is not working fine on it (see + # https://docs.python.org/fr/3/library/pty.html). + if self.env.build.os.name == "windows": + raise TestAbortWithError( + "Cannot run a pseudo-TTY on Windows systems" + ) + + # Ensure the process is run in the testsuite working dir + if not kwargs.get("cwd"): + kwargs["cwd"] = self.working_dir() + + # Open a subprocess with using a pseudo TTY as output + m, s = pty.openpty() + p = subprocess.Popen( + args=args, + stdout=s, + stderr=s, + close_fds=True, + **kwargs + ) + os.close(s) + + # Read result of the process execution and get its return code + fully_read = False + output = b"" + status_code = 0 + try: + while not fully_read: + ready, _, _ = select.select([m], [], [], 0.05) + for fd in ready: + try: + data = os.read(fd, 512) + except OSError as e: + if e.errno != errno.EIO: + raise e + fully_read = True + else: + if not data: + fully_read = True + output += data + finally: + os.close(m) + if p.poll() is None: + p.kill() + status_code = p.wait() + + return (output.decode(), status_code) + def parse_flagged_lines(self, output: str) -> Flags: """ Parse the output of a call and return an object containing files diff --git a/testsuite/drivers/gnatcheck_driver.py b/testsuite/drivers/gnatcheck_driver.py index 633483b4d..6be4c35ed 100644 --- a/testsuite/drivers/gnatcheck_driver.py +++ b/testsuite/drivers/gnatcheck_driver.py @@ -1,7 +1,6 @@ import os import os.path as P import re -import sys import xml.etree.ElementTree as ET from e3.testsuite.driver.diff import ( @@ -130,6 +129,8 @@ class GnatcheckDriver(BaseDriver): - ``worker``: Provide a custom worker for the GNATcheck run. - ``gnatkp_autoconfig`` (bool): Whether to automatically configure the target and runtime when running in "gnatkp" mode. Default is True. + - ``in_tty`` (bool): Whether to run GNATcheck in a pseudo TTY using the + ``pty`` Python module. - ``jobs`` (int): The number of jobs to forward to the GNATcheck command. - ``project`` (str): GPR build file to use (if any). @@ -404,10 +405,17 @@ def run_one_test(test_data: dict[str, any]) -> None: if label: self.output += label + "\n" + ("=" * len(label)) + "\n\n" - p = self.shell(args, env=gnatcheck_env, catch_error=False, analyze_output=False) - - # Get the GNATcheck execution output - exec_output = p.out + # Execute GNATcheck and get its output + exec_output = "" + status_code = 0 + if test_data.get("in_tty"): + exec_output, status_code = self.run_in_tty(args, env=gnatcheck_env) + else: + p = self.shell(args, env=gnatcheck_env, catch_error=False, analyze_output=False) + exec_output = p.out + status_code = p.status + + # Then read GNATcheck report file if there is one report_file_content = "" parse_output_for_flags = True if output_format in ['full', 'short', 'xml']: @@ -451,8 +459,8 @@ def run_one_test(test_data: dict[str, any]) -> None: self.output += ("testsuite_driver: Cannot found the rule " f"list file '{test_data['rule_list_file']}'") - if (not brief and p.status not in [0, 1]) or (brief and p.status != 0): - self.output += ">>>program returned status code {}\n".format(p.status) + if (not brief and status_code not in [0, 1]) or (brief and status_code != 0): + self.output += ">>>program returned status code {}\n".format(status_code) # List the content of directories if needed if test_data.get('list_dirs'): diff --git a/testsuite/tests/gnatcheck/lkql_rules_config/valid_rule_config/test.out b/testsuite/tests/gnatcheck/lkql_rules_config/valid_rule_config/test.out index bb38782e5..a90ad589e 100644 --- a/testsuite/tests/gnatcheck/lkql_rules_config/valid_rule_config/test.out +++ b/testsuite/tests/gnatcheck/lkql_rules_config/valid_rule_config/test.out @@ -1,4 +1,3 @@ -gnatcheck: The '-rules' section usage is discouraged from now, you should only use the '--rules' and '--rule-file' command-line options ada_code.adb:2:09: Int does not end with type suffix _T [first_convention|identifier_suffixes] ada_code.adb:5:09: Int_A does not end with access suffix _PTR [first_convention|identifier_suffixes] ada_code.adb:6:09: Int_PTR does not end with access suffix _A [other_convention|identifier_suffixes] diff --git a/testsuite/tests/gnatcheck/rules_depreciation_message/main.adb b/testsuite/tests/gnatcheck/rules_depreciation_message/main.adb new file mode 100644 index 000000000..9e86f10ac --- /dev/null +++ b/testsuite/tests/gnatcheck/rules_depreciation_message/main.adb @@ -0,0 +1,5 @@ +procedure Main is +begin + goto lbl; -- FLAG + <> +end Main; diff --git a/testsuite/tests/gnatcheck/rules_depreciation_message/test.out b/testsuite/tests/gnatcheck/rules_depreciation_message/test.out new file mode 100644 index 000000000..e63d2399f --- /dev/null +++ b/testsuite/tests/gnatcheck/rules_depreciation_message/test.out @@ -0,0 +1,3 @@ +gnatcheck: The '-rules' section is now deprecated. You should only use the '--rules' and '--rule-file' command-line options. +gnatcheck: You can use the '--emit-lkql-rule-file' flag to automatically translate your rule configuration to the new LKQL format. +main.adb:3:04: goto statement diff --git a/testsuite/tests/gnatcheck/rules_depreciation_message/test.yaml b/testsuite/tests/gnatcheck/rules_depreciation_message/test.yaml new file mode 100644 index 000000000..f1e21daf4 --- /dev/null +++ b/testsuite/tests/gnatcheck/rules_depreciation_message/test.yaml @@ -0,0 +1,11 @@ +driver: gnatcheck +format: brief +in_tty: True +rules: + - +RGoto_Statements +input_sources: + - main.adb +control: + - [SKIP, + "os == 'windows'", + "Disable this test because 'pty' is not supported on Windows"] diff --git a/testsuite/tests/gnatcheck_errors/same_name_instances/test.out b/testsuite/tests/gnatcheck_errors/same_name_instances/test.out index 92bfaee0b..286187064 100644 --- a/testsuite/tests/gnatcheck_errors/same_name_instances/test.out +++ b/testsuite/tests/gnatcheck_errors/same_name_instances/test.out @@ -17,7 +17,6 @@ gnatcheck: if you want to pass multiple parameters to a rule you should use the In command-line and rule options ================================ -gnatcheck: The '-rules' section usage is discouraged from now, you should only use the '--rules' and '--rule-file' command-line options gnatcheck: rule instance with the same name already exists: "goto_statements" previously instantiated at command line (rules.txt:1:1) gnatcheck: if you want to pass multiple parameters to a rule you should use the comma separated notation: e.g. +RMy_Rule:Param1,Param2 >>>program returned status code 5