Skip to content
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

checking that the system maxima has working help #36391

Merged
merged 3 commits into from
Oct 14, 2023

Conversation

dimpase
Copy link
Member

@dimpase dimpase commented Oct 3, 2023

This is needed for e.g. building Sage docs, cf
e.g. Sage's #36028. Some distos mix this up.

Fixes #36028

@dimpase
Copy link
Member Author

dimpase commented Oct 3, 2023

the puzzling pattern checked there is due to broken/no Maxima's help giving

$ echo ? ? | maxima 2>&1 | grep Couldn
(Details: CL-INFO::LOAD-PRIMARY-INDEX: Couldn't load

@antonio-rojas
Copy link
Contributor

Even though this makes sense, this should be fixed at the docker CI level first as noted in #36028 (comment). Otherwise building on Arch with system maxima will no longer be tested.

@dimpase
Copy link
Member Author

dimpase commented Oct 3, 2023

should I change Couldn to Details: ?
(on assumption that this is a part of any LISP error message)

@dimpase
Copy link
Member Author

dimpase commented Oct 3, 2023

Even though this makes sense, this should be fixed at the docker CI level first as noted in #36028 (comment). Otherwise building on Arch with system maxima will no longer be tested.

I don't see where the image is installed in CI. There are a number of places archlinux is mentioned, but no idea where
one can sneak in commands to modify the running image (to install Maxima with help)

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 3, 2023

I don't see where the image is installed in CI. There are a number of places archlinux is mentioned, but no idea where
one can sneak in commands to modify the running image (to install Maxima with help)

build/bin/write-dockerfile.sh

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

Documentation preview for this PR (built with commit 7cd0cb6; changes) is ready! 🎉

@dimpase
Copy link
Member Author

dimpase commented Oct 8, 2023

I don't see where the image is installed in CI. There are a number of places archlinux is mentioned, but no idea where
one can sneak in commands to modify the running image (to install Maxima with help)

build/bin/write-dockerfile.sh

can we handle this in another, archlinux-specific, PR?

@dimpase
Copy link
Member Author

dimpase commented Oct 8, 2023

I don't see where the image is installed in CI. There are a number of places archlinux is mentioned, but no idea where
one can sneak in commands to modify the running image (to install Maxima with help)

build/bin/write-dockerfile.sh

Would

--- a/build/bin/write-dockerfile.sh
+++ b/build/bin/write-dockerfile.sh
@@ -104,6 +104,7 @@ EOF
         UPDATE="pacman -Sy &&"
         EXISTS="pacman -Si"
         INSTALL="pacman -Su --noconfirm"
+       RUN="RUN sed -i '/^NoExtract/d' /etc/pacman.conf"
         ;;
     nix*)
         # https://hub.docker.com/r/nixos/nix

do the trick, or I misunderstand what RUN does?

@dimpase
Copy link
Member Author

dimpase commented Oct 8, 2023

as far as I can see, test on arch fails due to missing container:

Step 1/44 : ARG BASE_IMAGE
Step 2/44 : FROM ${BASE_IMAGE} as with-system-packages
manifest unknown
Error response from daemon: No such container: ghcr.io/sagemath/sage/sage-archlinux-standard-sitepackages-with-targets:10.2.beta5-6-g6bb5776cad-failed
Pushing ghcr.io/sagemath/sage/sage-archlinux-standard-sitepackages-with-targets:10.2.beta5-6-g6bb5776cad-failed
The push refers to repository [ghcr.io/sagemath/sage/sage-archlinux-standard-sitepackages-with-targets]

This is needed for e.g. building Sage docs, cf
e.g. Sage's sagemath#36028. Some distos mix this up.

This fix was requested there.
@mkoeppe mkoeppe force-pushed the check_system_maxima_help branch from 1066b44 to ef67ba0 Compare October 8, 2023 18:58
@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 8, 2023

I misunderstand what RUN does?

I've fixed it for you

@dimpase
Copy link
Member Author

dimpase commented Oct 8, 2023

I don't understand what it does. Does it create a file? A script to run?

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 8, 2023

The script writes a Dockerfile to stdout

@dimpase
Copy link
Member Author

dimpase commented Oct 9, 2023

Hmm, and why it's not just using echo to dump these RUN ... lines?

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 9, 2023

why it's not just using echo to dump these RUN ... lines?

That would be equivalent, of course, but this file uses HERE docs for various multi-line snippets, and so it also does single-line snippets in the same way for consistency.

@vbraun vbraun merged commit d0073b2 into sagemath:develop Oct 14, 2023
@mkoeppe mkoeppe added this to the sage-10.2 milestone Oct 14, 2023
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 13, 2024
sagemathgh-36959: `.devcontainer/downstream-archlinux-latest`: Fix maxima
    
Dev container configurations `.devcontainer/downstream-*` give easy
access to Sage as provided by downstream packagers (see https://doc.sage
math.org/html/en/developer/portability_testing.html#using-our-pre-built-
docker-images-for-development-in-vs-code, bottom).

Unfortunately maxima is defective when the archlinux sage package is
installed in the official archlinux Docker container because of
NoExtract directives that make the maxima help system inoperable.

Here we apply the same fix that we used in sagemath#36391 for the portability
CI: Removing the NoExtract directives before installing the package.

<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36959
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

archlinux-latest-standard: System maxima does not work
4 participants