Skip to content

singular spkg-configure.m4: Better test for help #33390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mkoeppe opened this issue Feb 20, 2022 · 15 comments
Closed

singular spkg-configure.m4: Better test for help #33390

mkoeppe opened this issue Feb 20, 2022 · 15 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 20, 2022

As reported in https://groups.google.com/g/sage-release/c/vxYHQA6LUGw/m/_LO2GLLlAAAJ, the current test is not suitable on some systems.

CC: @dimpase @strogdon @orlitzky

Component: build: configure

Author: Steven Trogdon

Branch/Commit: 645a88b

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/33390

@mkoeppe mkoeppe added this to the sage-9.6 milestone Feb 20, 2022
@strogdon
Copy link
Contributor

comment:1

Perhaps something like

diff --git a/build/pkgs/singular/spkg-configure.m4 b/build/pkgs/singular/spkg-configure.m4
index 0f629e4d31..95a67f79bc 100644
--- a/build/pkgs/singular/spkg-configure.m4
+++ b/build/pkgs/singular/spkg-configure.m4
@@ -6,7 +6,7 @@ SAGE_SPKG_CONFIGURE([singular], [
       dnl Use pkg-config to ensure that Singular is new enough.
       PKG_CHECK_MODULES([SINGULAR], [Singular >= 4.2.1], [
        AC_MSG_CHECKING([that Singular's help is working])
-       AS_IF([test x`echo "help;" | Singular 2>&1 | grep "error\ occurred"` = x], [
+       AS_IF([test x`echo -e "system(\"--browser\", \"builtin\"); \n help;" | Singular 2>&1 | grep "error\ occurred"` = x], [
         AC_MSG_RESULT(yes)
         dnl We have Singular. Now determine the shared library path on
         dnl platforms on which sage.libs.singular needs to reload the library with RTLD_GLOBAL.

will work?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 21, 2022

comment:2

Yes, I think this would work

@strogdon
Copy link
Contributor

Branch: u/strogdon/trac_33390

@strogdon
Copy link
Contributor

comment:4

Let's make sure this is safe on other distros.


New commits:

1d8cf65Update test for help docs in Singular

@strogdon
Copy link
Contributor

Commit: 1d8cf65

@strogdon
Copy link
Contributor

Author: Steven Trogdon

@strogdon
Copy link
Contributor

comment:5

Replying to @strogdon:

Let's make sure this is safe on other distros.


New commits:

1d8cf65 Update test for help docs in Singular
It seems good on Gentoo.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 21, 2022

comment:6

tox -e docker-ubuntu-jammy-standard -- config.status also looks fine.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 21, 2022

Reviewer: Matthias Koeppe

@orlitzky
Copy link
Contributor

comment:7

Can you please use printf instead of echo here? The -e flag and escape sequences won't necessarily work with non-bash shells.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 22, 2022

Changed commit from 1d8cf65 to 645a88b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 22, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

645a88bUse `printf` instead of `echo -e`

@strogdon
Copy link
Contributor

comment:9

@orlitzky let me know if not as expected. I'm so accustomed to using bash that I forget there are other shells.

@orlitzky
Copy link
Contributor

comment:10

It worked in dash, zsh, and ksh; so good enough for me. Thanks!

@vbraun
Copy link
Member

vbraun commented Feb 27, 2022

Changed branch from u/strogdon/trac_33390 to 645a88b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants