From ffe23f9485adf2aff8636345f00182ef06331803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Labb=C3=A9?= Date: Fri, 14 Jan 2022 12:32:55 +0100 Subject: [PATCH 1/5] 33167: adding a second check in lrs feature is_functional --- src/sage/features/lrs.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/sage/features/lrs.py b/src/sage/features/lrs.py index 4defebd069f..0ed9649cd04 100644 --- a/src/sage/features/lrs.py +++ b/src/sage/features/lrs.py @@ -43,6 +43,8 @@ def is_functional(self): FeatureTestResult('lrslib', True) """ from sage.misc.temporary_file import tmp_filename + + # Check #1 tf_name = tmp_filename() with open(tf_name, 'wb') as tf: tf.write(str_to_bytes("V-representation\nbegin\n 1 1 rational\n 1 \nend\nvolume")) @@ -62,6 +64,22 @@ def is_functional(self): command=" ".join(command), expected=" or ".join(expected_list))) + # Check #2 + # Checking whether `lrsnash` can handle the new input format + # This test is currently done in build/pkgs/lrslib/spkg-configure.m4 + tf_name = tmp_filename() + with open(tf_name, 'w') as tf: + tf.write("1 1\n \n 0\n \n 0\n") + command = ['lrsnash', tf_name] + result = subprocess.run(command, capture_output=True, text=True) + if result.returncode: + return FeatureTestResult(self, False, reason='Running command "{}" ' + 'returned non-zero exit status "{}" with stderr ' + '"{}" and stdout "{}".'.format(' '.join(result.args), + result.returncode, + result.stderr.strip(), + result.stdout.strip())) + return FeatureTestResult(self, True) From 986ed5305d3b0827da4c6699928fae87a706ae5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Labb=C3=A9?= Date: Fri, 14 Jan 2022 12:43:52 +0100 Subject: [PATCH 2/5] 33167: simplification of check #1 in is_functional method of lrs feature --- src/sage/features/lrs.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/sage/features/lrs.py b/src/sage/features/lrs.py index 0ed9649cd04..79decd3e175 100644 --- a/src/sage/features/lrs.py +++ b/src/sage/features/lrs.py @@ -3,11 +3,9 @@ Feature for testing the presence of ``lrslib`` """ -import os import subprocess from . import Executable, FeatureTestResult -from sage.cpython.string import str_to_bytes, bytes_to_str class Lrs(Executable): @@ -46,19 +44,17 @@ def is_functional(self): # Check #1 tf_name = tmp_filename() - with open(tf_name, 'wb') as tf: - tf.write(str_to_bytes("V-representation\nbegin\n 1 1 rational\n 1 \nend\nvolume")) - devnull = open(os.devnull, 'wb') + with open(tf_name, 'w') as tf: + tf.write("V-representation\nbegin\n 1 1 rational\n 1 \nend\nvolume") command = ['lrs', tf_name] - try: - lines = bytes_to_str(subprocess.check_output(command, stderr=devnull)) - except subprocess.CalledProcessError as e: + result = subprocess.run(command, capture_output=True, text=True) + if result.returncode: return FeatureTestResult(self, False, - reason="Call to `{command}` failed with exit code {e.returncode}.".format(command=" ".join(command), e=e)) + reason="Call to `{command}` failed with exit code {result.returncode}.".format(command=" ".join(command), result=result)) expected_list = ["Volume= 1", "Volume=1"] - if all(lines.find(expected) == -1 for expected in expected_list): - print(lines) + if all(result.stdout.find(expected) == -1 for expected in expected_list): + print(result.stdout) return FeatureTestResult(self, False, reason="Output of `{command}` did not contain the expected result {expected}.".format( command=" ".join(command), From ab57121e38ec952845da6f1d0c953001389d4883 Mon Sep 17 00:00:00 2001 From: Markus Wageringel Date: Mon, 17 Jan 2022 20:59:35 +0100 Subject: [PATCH 3/5] 33167: avoid printing of output --- src/sage/features/lrs.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sage/features/lrs.py b/src/sage/features/lrs.py index 79decd3e175..7031982797b 100644 --- a/src/sage/features/lrs.py +++ b/src/sage/features/lrs.py @@ -54,11 +54,11 @@ def is_functional(self): expected_list = ["Volume= 1", "Volume=1"] if all(result.stdout.find(expected) == -1 for expected in expected_list): - print(result.stdout) return FeatureTestResult(self, False, - reason="Output of `{command}` did not contain the expected result {expected}.".format( + reason="Output of `{command}` did not contain the expected result {expected}; output: {result.stdout}".format( command=" ".join(command), - expected=" or ".join(expected_list))) + expected=" or ".join(expected_list), + result=result)) # Check #2 # Checking whether `lrsnash` can handle the new input format From ed2a3d4d17e004d5f2ebc40d64f8f5c74e7628f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Labb=C3=A9?= Date: Thu, 27 Jan 2022 09:54:33 +0100 Subject: [PATCH 4/5] 33167: add try except clause to catch OSError during run --- src/sage/features/lrs.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/sage/features/lrs.py b/src/sage/features/lrs.py index 7031982797b..f6a4d922bd0 100644 --- a/src/sage/features/lrs.py +++ b/src/sage/features/lrs.py @@ -47,7 +47,12 @@ def is_functional(self): with open(tf_name, 'w') as tf: tf.write("V-representation\nbegin\n 1 1 rational\n 1 \nend\nvolume") command = ['lrs', tf_name] - result = subprocess.run(command, capture_output=True, text=True) + try: + result = subprocess.run(command, capture_output=True, text=True) + except OSError as e: + return FeatureTestResult(self, False, reason='Running command "{}" ' + 'raised an OSError "{}" '.format(' '.join(command), e)) + if result.returncode: return FeatureTestResult(self, False, reason="Call to `{command}` failed with exit code {result.returncode}.".format(command=" ".join(command), result=result)) @@ -67,7 +72,11 @@ def is_functional(self): with open(tf_name, 'w') as tf: tf.write("1 1\n \n 0\n \n 0\n") command = ['lrsnash', tf_name] - result = subprocess.run(command, capture_output=True, text=True) + try: + result = subprocess.run(command, capture_output=True, text=True) + except OSError as e: + return FeatureTestResult(self, False, reason='Running command "{}" ' + 'raised an OSError "{}" '.format(' '.join(command), e)) if result.returncode: return FeatureTestResult(self, False, reason='Running command "{}" ' 'returned non-zero exit status "{}" with stderr ' From 16e1b241bff8e0485f773c7b420fd7b2fc4defa5 Mon Sep 17 00:00:00 2001 From: Markus Wageringel Date: Mon, 17 Jan 2022 21:15:27 +0100 Subject: [PATCH 5/5] 33167: add comment about polymake's use of lrslib --- build/pkgs/lrslib/spkg-configure.m4 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build/pkgs/lrslib/spkg-configure.m4 b/build/pkgs/lrslib/spkg-configure.m4 index 0297f7118e3..752dcfe8fbf 100644 --- a/build/pkgs/lrslib/spkg-configure.m4 +++ b/build/pkgs/lrslib/spkg-configure.m4 @@ -1,4 +1,7 @@ SAGE_SPKG_CONFIGURE([lrslib], [ + dnl Although lrs and lrsnash binaries are the only interfaces to lrslib that + dnl sagelib uses, the optional polymake package uses lrslib as a library, so + dnl the following DEPCHECK is needed. dnl System lrslib may already be 7.x, which may be compiled with FLINT SAGE_SPKG_DEPCHECK([gmp flint], [ AC_CHECK_PROGS([LRSNASH], [lrsnash])