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

System package information tox ini gh actions for alpine linux #35285

Conversation

dimpase
Copy link
Member

@dimpase dimpase commented Mar 14, 2023

📚 system pkgs info and GH actions for alpine linux

Will fix #33083. This is the rebase of the branch of the latter on top of 10.0.beta4, reduced to the scripts and package lists.

Workarounds for musl libc from #33083 will be revisited in a follow up.

📝 Checklist

  • I have made sure that the title is self-explanatory and the description concisely explains the PR.
  • I have linked an issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

Dependencies

@dimpase dimpase marked this pull request as draft March 14, 2023 16:19
@dimpase
Copy link
Member Author

dimpase commented Mar 14, 2023

$ tox -e docker-alpine-standard
...
Sending build context to Docker daemon  448.4MB
Step 1/101 : ARG BASE_IMAGE
Step 2/101 : FROM ${BASE_IMAGE} as with-system-packages
 ---> b2aa39c304c2
Step 3/101 : ARG BASE_IMAGE
 ---> Using cache
 ---> f2c2373f70e0
with-system-packages stage name already used
sha256:cdad8a88ed941bb803bea2211e3aa146a33df6a0e504b0c22bd1c3e14764c85f
docker-alpine-standard: exit 1 (11.68 seconds) /Users/dima/software/sage> bash -c 'for docker_target in with-targets; do BUILD_IMAGE_STEM=sage-$(echo docker-alpine-standard | sed "s/docker-//;s/-incremental//"); BUILD_IMAGE=$DOCKER_PUSH_REPOSITORY$BUILD_IMAGE_STEM-$docker_target; BUILD_TAG=$(git describe --dirty --always); TAG_ARGS=$(for tag in $BUILD_TAG ; do echo --tag $BUILD_IMAGE:$tag; done); DOCKER_BUILDKIT=0 docker build . -f /Users/dima/software/sage/.tox/docker-alpine-standard/Dockerfile --target $docker_target $TAG_ARGS --build-arg EXTRA_CONFIGURE_ARGS="--enable-experimental-packages --enable-download-from-upstream-url  --with-system-python3=yes  " --build-arg BASE_IMAGE=alpine:latest --build-arg BOOTSTRAP="./bootstrap" --build-arg TARGETS_PRE="$(if test -n "$TARGETS_PRE"; then echo $TARGETS_PRE; else echo all-sage-local; fi)" --build-arg TARGETS="build" --build-arg TARGETS_OPTIONAL="ptest" ; status=$?; if [ $status != 0 ]; then BUILD_TAG="$BUILD_TAG-failed"; docker commit $(docker ps -l -q) $BUILD_IMAGE:$BUILD_TAG; PUSH_TAGS=$BUILD_IMAGE:$BUILD_TAG; else PUSH_TAGS=$(echo $BUILD_IMAGE:$BUILD_TAG; for tag in ; do echo "$BUILD_IMAGE:$tag"; done); fi; echo $BUILD_IMAGE:$BUILD_TAG >> /Users/dima/software/sage/.tox/docker-alpine-standard/Dockertags; if [ x"" != x ]; then echo Pushing $PUSH_TAGS; for tag in $PUSH_TAGS; do docker push $tag || echo "(ignoring errors)"; done; fi; if [ $status != 0 ]; then exit $status; fi; done' pid=90915
  docker-alpine-standard: FAIL code 1 (12.66=setup[0.08]+cmd[0.89,0.01,11.68] seconds)
  evaluation failed :( (12.80 seconds)

@mkoeppe - do you know what this means? a duplicate stage name? just rename?

@dimpase dimpase requested a review from mkoeppe March 14, 2023 16:23
@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 14, 2023

do you know what this means? a duplicate stage name? just rename?

Sounds like a bug in build/bin/write-dockerfile.sh for alpine. Have you looked at the generated Dockerfile that is created in .tox/....?

@dimpase
Copy link
Member Author

dimpase commented Mar 15, 2023

On a standalone apline container, pplpy fails to build (this is gcc 12.2.1):

[pplpy-0.8.6]   ppl/polyhedron.cpp: In function 'PyObject* __pyx_pf_3ppl_10polyhedron_10Polyhedron_96OK(__pyx_obj_3ppl_10polyhedron_Polyhedron*, PyObject*)':
[pplpy-0.8.6]   ppl/polyhedron.cpp:10298:46: error: converting to 'bool' from 'std::nullptr_t' requires direct-initialization [-fpermissive]
[pplpy-0.8.6]   10298 |   __pyx_v_result = __pyx_v_self->thisptr->OK(NULL);
[pplpy-0.8.6]         |                                              ^~~~
[pplpy-0.8.6]   /sage/local/include/ppl.hh:48056:16: note:   initializing argument 1 of 'bool Parma_Polyhedra_Library::Polyhedron::OK(bool) const'
[pplpy-0.8.6]   48056 |   bool OK(bool check_not_empty = false) const;
[pplpy-0.8.6]         |           ~~~~~^~~~~~~~~~~~~~~~~~~~~~~
[pplpy-0.8.6]   error: command '/usr/bin/gcc' failed with exit code 1
[pplpy-0.8.6]   error: subprocess-exited-with-error
[pplpy-0.8.6]   
[pplpy-0.8.6]   × Building wheel for pplpy (pyproject.toml) did not run successfully.

the offending code has been removed in pplpy's master:
sagemath/pplpy@8ca5bfe#diff-0072b6af459c8f91709f019ccd543cfefe53e779281b392433fdf1010facd937
(line 2356).

So we need new pplpy release to progress, it seems.

@dimpase
Copy link
Member Author

dimpase commented Mar 15, 2023

strangely, Sage has pplpy 0.8.6, but there is also 0.8.7, released 2 days later, cf
https://pypi.org/project/pplpy/#history

@dimpase
Copy link
Member Author

dimpase commented Mar 15, 2023

With pplpy 0.8.7, pplpy builds (see #35290 - should be dependence of this PR).

Also, giac build needed nls disabled - but perhaps I didn't set locale properly
(it complained about not able to link libintl_gettext, even though libintl is installed)

--- a/build/pkgs/giac/spkg-install.in
+++ b/build/pkgs/giac/spkg-install.in
@@ -48,7 +48,8 @@ echo "Configuring giac..."
 
 #    --disable-ao     (avoid libao deps)
 #   On OS X (10.12) the built in intl is broken
-DISABLENLS=""
+# DISABLENLS=""
+DISABLENLS="--disable-nls"
 if [ "$UNAME" = "Darwin" ]; then
     echo "OS X Building without Native Language Support"
     DISABLENLS="--disable-nls"

@dimpase
Copy link
Member Author

dimpase commented Mar 15, 2023

on alpine, suitesparse installs its headers in /usr/include/suitesparse/, and -I /usr/include/suitesparse/ has to be added to CFLAGS for cvxopt to build successfully.

@dimpase
Copy link
Member Author

dimpase commented Mar 15, 2023

and then, building giac extension fails with a gammaf vs tgammaf issue:

[sagelib-10.0.beta4]     running build_ext
[sagelib-10.0.beta4]     building 'sage.libs.giac.giac' extension
[sagelib-10.0.beta4]     INFO: C compiler: gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -Os -fomit-frame-pointer -g -O2 -Os -fomit-frame-pointer -g -O2 -Os -fomit-frame-pointer -g -
O2 -DTHREAD_STACK_SIZE=0x100000 -g -O2 -fPIC
[sagelib-10.0.beta4] 
[sagelib-10.0.beta4]     INFO: compile options: '-Isage/libs/giac -I/sage/local/var/lib/sage/venv-python3.10/lib/python3.10/site-packages/cysignals -Isage/cpython -I/sage/src -I/sage/local/var/lib/sage/ve
nv-python3.10/lib/python3.10/site-packages/numpy/core/include -I/usr/include/python3.10 -I/sage/local/var/lib/sage/venv-python3.10/include -I/usr/include/python3.10 -c'
[sagelib-10.0.beta4]     INFO: gcc: sage/libs/giac/giac.cpp
[sagelib-10.0.beta4]     In file included from /sage/local/include/giac/poly.h:25,
[sagelib-10.0.beta4]                      from /sage/local/include/giac/giac.h:5,
[sagelib-10.0.beta4]                      from sage/libs/giac/giac.cpp:802:
[sagelib-10.0.beta4]     /sage/local/include/giac/index.h:33: warning: ignoring '#pragma anon_unions ' [-Wunknown-pragmas]
[sagelib-10.0.beta4]        33 | #pragma anon_unions
[sagelib-10.0.beta4]           |
[sagelib-10.0.beta4]     In file included from /sage/local/include/giac/giac.h:3:
[sagelib-10.0.beta4]     /sage/local/include/giac/first.h: In function 'float fgamma(float)':
[sagelib-10.0.beta4]     /sage/local/include/giac/first.h:568:39: error: 'gammaf' was not declared in this scope; did you mean 'tgammaf'?
[sagelib-10.0.beta4]       568 | inline float fgamma(float f1){ return gammaf(f1); } // or tgammaf(f1) on some versions of emscripten
[sagelib-10.0.beta4]           |                                       ^~~~~~
[sagelib-10.0.beta4]           |                                       tgammaf
[sagelib-10.0.beta4]     /sage/local/include/giac/index.h: In copy constructor 'giac::index_m::index_m(const giac::index_m&)':
[sagelib-10.0.beta4]     /sage/local/include/giac/index.h:365:11: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
[sagelib-10.0.beta4]       365 |         * (size_t *) & taille = * (size_t *) &im.taille;
[sagelib-10.0.beta4]           |           ^~~~~~~~~~~~~~~~~~~
[sagelib-10.0.beta4]     /sage/local/include/giac/index.h:365:35: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
[sagelib-10.0.beta4]       365 |         * (size_t *) & taille = * (size_t *) &im.taille;
[sagelib-10.0.beta4]           |                                   ^~~~~~~~~~~~~~~~~~~~~
[sagelib-10.0.beta4]     /sage/local/include/giac/index.h:367:11: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
[sagelib-10.0.beta4]       367 |         * (size_t *) other = * (size_t *) im.other;
[sagelib-10.0.beta4]           |           ^~~~~~~~~~~~~~~~

this is probably musl problem

@dimpase dimpase requested a review from tornaria March 15, 2023 16:47
@dimpase
Copy link
Member Author

dimpase commented Mar 15, 2023

Maybe Gonzalo has a musl-specific advice on the gammaf problem.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 15, 2023

The original ticket #33083 has a lot of discussion on that already

@tornaria
Copy link
Contributor

Maybe Gonzalo has a musl-specific advice on the gammaf problem.

See: https://github.com/void-linux/void-packages/tree/master/srcpkgs/giac/patches

Note that gammaf is deprecated because it used to mean the gamma function itself or its logarithm in different systems. C99 and posix.2001 standarizes tgammaf (true gamma) and lgammaf (log gamma) so this is 20 years old.

I think for musl you also need to apply at least the malloc patch, and possibly LDFLAGS="-Wl,-z,stack-size=2097152" from https://github.com/void-linux/void-packages/blob/master/srcpkgs/giac/template.

For giac 1.9.0-41 I also had to touch doc/en/tutoriel.stamp and I started doing sed -i configure.ac -e '/m4_define(\[giac_micro_version]/ s/\[0]/[0-41]/' (where 41 is actually computed from the pkgver) since upstream is too lazy to change the version banner at every release 🤦‍♂️ .

The fact that we still need to carry patches for pari 2.11 which upstream refuses to merge is not very encouraging. Who is still using pari 2.9?

For pplpy I'm using 0.8.7 without patches.

@dimpase
Copy link
Member Author

dimpase commented Mar 15, 2023

turns out also #35094 is needed - otherwise libsingular linking is broken

@tornaria
Copy link
Contributor

turns out also #35094 is needed - otherwise libsingular linking is broken

Yes, because find_library() in python doesn't support musl. We have a patch for python from alpine, so maybe using system python in alpine works. But #35094 is designed to avoid this issue.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 18, 2023

Let's just get this in? Here we are just adding testing for a platform; it is not a promise of support.

@mkoeppe mkoeppe marked this pull request as ready for review June 18, 2023 15:43
Copy link
Contributor

@mkoeppe mkoeppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve @dimpase's commits

@mkoeppe mkoeppe force-pushed the system_package_information___tox_ini___gh_actions_for_alpine_linux branch from 8e16d13 to 069e063 Compare July 9, 2023 01:24
@dimpase dimpase force-pushed the system_package_information___tox_ini___gh_actions_for_alpine_linux branch from 069e063 to 4dc363d Compare September 28, 2023 20:17
@dimpase
Copy link
Member Author

dimpase commented Sep 28, 2023

the label positive review was not put up on time. Oh well, better late than never

@mkoeppe
Copy link
Contributor

mkoeppe commented Sep 28, 2023

Thanks!

@vbraun
Copy link
Member

vbraun commented Oct 1, 2023

On fedora 38 / gcc 13:

[arb-2.22.1] eig_multiple_rump.c:26:1: error: conflicting types for 'close'; have 'int(const acb_struct *, const acb_struct *, const mag_struct *)'
[arb-2.22.1]    26 | close(const acb_t x, const acb_t y, const mag_t eps)
[arb-2.22.1]       | ^~~~~
[arb-2.22.1] In file included from /usr/include/bits/sigstksz.h:24,
[arb-2.22.1]                  from /usr/include/signal.h:328,
[arb-2.22.1]                  from /usr/include/sys/param.h:28,
[arb-2.22.1]                  from ../../../../../../../include/flint/flint.h:18,
[arb-2.22.1]                  from ../../../../../../../include/flint/fmpz_mat.h:29,
[arb-2.22.1]                  from ../acb_mat.h:22,
[arb-2.22.1]                  from eig_multiple_rump.c:12:
[arb-2.22.1] /usr/include/unistd.h:358:12: note: previous declaration of 'close' with type 'int(int)'
[arb-2.22.1]   358 | extern int close (int __fd);
[arb-2.22.1]       |            ^~~~~
[arb-2.22.1] make[8]: *** [../Makefile.subdirs:60: ../build/acb_mat/eig_multiple_rump.lo] Error 1
[arb-2.22.1] make[8]: *** Waiting for unfinished jobs....

@dimpase
Copy link
Member Author

dimpase commented Oct 1, 2023

On fedora 38 / gcc 13:

[arb-2.22.1] eig_multiple_rump.c:26:1: error: conflicting types for 'close'; have 'int(const acb_struct *, const acb_struct *, const mag_struct *)'
[arb-2.22.1]    26 | close(const acb_t x, const acb_t y, const mag_t eps)
[arb-2.22.1]       | ^~~~~
[arb-2.22.1] In file included from /usr/include/bits/sigstksz.h:24,
[arb-2.22.1]                  from /usr/include/signal.h:328,
[arb-2.22.1]                  from /usr/include/sys/param.h:28,
[arb-2.22.1]                  from ../../../../../../../include/flint/flint.h:18,
[arb-2.22.1]                  from ../../../../../../../include/flint/fmpz_mat.h:29,
[arb-2.22.1]                  from ../acb_mat.h:22,
[arb-2.22.1]                  from eig_multiple_rump.c:12:
[arb-2.22.1] /usr/include/unistd.h:358:12: note: previous declaration of 'close' with type 'int(int)'
[arb-2.22.1]   358 | extern int close (int __fd);
[arb-2.22.1]       |            ^~~~~
[arb-2.22.1] make[8]: *** [../Makefile.subdirs:60: ../build/acb_mat/eig_multiple_rump.lo] Error 1
[arb-2.22.1] make[8]: *** Waiting for unfinished jobs....

@vbraun - why do you think this is relevant here? This ticket specifically deals with Apline Linux, it doesn't touch Fedora at all!

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 1, 2023

The branch does contain a change to arb's spkg-install.

@dimpase
Copy link
Member Author

dimpase commented Oct 1, 2023

we unconditionally change CFLAGS for flint and arb here. Hmm, should we only do this on Alpine and other musl systems?

We can write a test along the lines of #33083 (comment)

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 1, 2023

May be worth checking whether it's already fixed upstream

@dimpase
Copy link
Member Author

dimpase commented Oct 1, 2023

May be worth checking whether it's already fixed upstream

The fix should be in Flint 2.9, but we are still on 2.8.4

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 1, 2023

Then let's back out this change to spkg-install here so that we're really only adding those .txt files here.

And in a separate PR let's revive the FLINT/arb upgrades #34102/#34106

@mkoeppe mkoeppe force-pushed the system_package_information___tox_ini___gh_actions_for_alpine_linux branch from 4dc363d to bb35159 Compare October 2, 2023 01:03
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

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

Copy link
Member Author

@dimpase dimpase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@vbraun vbraun merged commit 1ed5b51 into sagemath:develop Oct 8, 2023
23 of 28 checks passed
@mkoeppe mkoeppe added this to the sage-10.2 milestone Oct 8, 2023
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.

alpine linux: Add system package information / tox.ini, fix flint, giac, cvxopt
4 participants