#30729 closed defect (fixed)

Fix gap_packages for Xcode 12

Reported by: mkoeppe Owned by:
Priority: critical Milestone: sage-9.3
Component: packages: optional Keywords:
Cc: dimpase, jhpalmieri, soehms Merged in:
Authors: Matthias Koeppe, John Palmieri Reviewers: Dima Pasechnik
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: d8f07f9 (Commits, GitHub, GitLab) Commit: d8f07f93630c5bac197f3b525a5cbb2ab3fa096e
Dependencies: #30720 Stopgaps:

Status badges

Description (last modified by jhpalmieri)

Follow-up from #30720. Building gap_packages (version 4.10.2) using Xcode 12 fails:

gcc -c -O -fno-builtin gpd.c 
gpd.c:37:7: error: implicit declaration of function 'gpprog' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  if (gpprog()== -1) exit(1);
      ^
1 error generated.

Change History (33)

comment:1 Changed 12 months ago by mkoeppe

  • Dependencies set to #30720

comment:2 Changed 12 months ago by dimpase

  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

comment:3 Changed 12 months ago by mkoeppe

Was the update ticket #29314 successfully tested on Xcode 12?

comment:4 Changed 12 months ago by dimpase

it might be I didn't test the whole of gap_packages. I thought I did.

comment:5 follow-up: Changed 12 months ago by jhpalmieri

I don't think #29314 upgrades gap_packages, but the new gap and libsemigroups build for me.

comment:6 in reply to: ↑ 5 Changed 12 months ago by mkoeppe

Replying to jhpalmieri:

I don't think #29314 upgrades gap_packages

It does because it uses the same tarball as gap, via symlinks in build/pkgs/gap_packages

comment:7 Changed 12 months ago by mkoeppe

As suggested in #29314, patching the failing packages using the diff to the GAP 4.11 distribution would be a way forward.

comment:8 Changed 12 months ago by mkoeppe

  • Reviewers set to https://github.com/mkoeppe/sage/actions/runs/289319582

comment:9 Changed 12 months ago by mkoeppe

The failure is now also reproduced on GH Actions in local-homebrew-usrlocal-macos-maximal-xcode12, https://github.com/mkoeppe/sage/runs/1208931537

Last edited 12 months ago by mkoeppe (previous) (diff)

comment:10 Changed 12 months ago by mkoeppe

  • Branch set to u/mkoeppe/fix_gap_packages_for_xcode_12

comment:11 follow-up: Changed 12 months ago by jhpalmieri

  • Commit set to 8693dc75891529b7e7e349c8ac4e1ac4076ea785
  • Description modified (diff)

I see the same error with this branch.


New commits:

c26eb00fix cohomolo pkg code to allow build with gcc10
102946dbuild/pkgs/gap_packages/patches/cohomolo-gcc10.patch: Backport
2c30b34clang 12 (macOS XCode 12) needs extra includes
f89dc20build/pkgs/gap_packages/patches/guava_leon_includes.patch: Backport
1b62f42Merge branch 'u/mkoeppe/fix_gap_packages_for_gcc_10__xcode_12' of git://trac.sagemath.org/sage into t/30729/fix_gap_packages_for_xcode_12
a98639e.github/workflows/tox.yml: Test homebrew-usrlocal in addition to homebrew
8693dc7.github/workflows/tox-{optional,experimental}.yml: Test homebrew-usrlocal instead of homebrew

comment:12 in reply to: ↑ 11 Changed 12 months ago by dimpase

Replying to jhpalmieri:

I see the same error with this branch.

in GAP 4.10 the cohomolo's Makefile.in does not have a line

EXTRA_CFLAGS += -Wno-implicit-function-declaration

which is present in GAP 4.11 - so probably the quickest patch would be to add it in GAP 4.10. (I started on fixing the code itself, but it's a nontrivial amount of changes).

comment:13 Changed 12 months ago by dimpase

Even quicker should be to do

  • build/pkgs/gap_packages/spkg-install.in

    diff --git a/build/pkgs/gap_packages/spkg-install.in b/build/pkgs/gap_packages/spkg-install.in
    index 10b444b5c6..cc094ad40c 100644
    a b PKG_DIR="$GAP_ROOT/pkg" 
    33
    44PKG_SRC_DIR="$(pwd)/src/pkg"
    55cd "$PKG_SRC_DIR"
     6CFLAGS = "$CFLAGS -Wno-implicit-function-declaration"
     7export CFLAGS
    68
    79# directly install pure GAP packages
    810#

comment:14 follow-up: Changed 12 months ago by jhpalmieri

I tried this, and it doesn't work for me: same error as before.

comment:15 in reply to: ↑ 14 Changed 12 months ago by dimpase

Replying to jhpalmieri:

I tried this, and it doesn't work for me: same error as before.

Could we see the full error, with the corresponding compiler call?

comment:16 Changed 12 months ago by dimpase

the old Makefile.in there needs a patch, in fact, that's why it does not work

comment:17 follow-up: Changed 12 months ago by jhpalmieri

This works for me, instead:

  • build/pkgs/gap_packages/spkg-install.in

    diff --git a/build/pkgs/gap_packages/spkg-install.in b/build/pkgs/gap_packages/spkg-install.in
    index 10b444b5c6..56c444e11f 100644
    a b install_compiled_pkg() 
    6868for pkg in cohomolo-* crypting-* grape-* guava-* orb-*
    6969do
    7070    echo "Building GAP package $pkg"
     71    CFLAGS="$CFLAGS -Wno-implicit-function-declaration"
     72    export CFLAGS
    7173    cd "$PKG_SRC_DIR/$pkg"
    7274    ./configure "$GAP_ROOT"
    7375    sdh_make -j1

comment:18 in reply to: ↑ 17 Changed 12 months ago by dimpase

Replying to jhpalmieri:

This works for me, instead:

  • build/pkgs/gap_packages/spkg-install.in

    diff --git a/build/pkgs/gap_packages/spkg-install.in b/build/pkgs/gap_packages/spkg-install.in
    index 10b444b5c6..56c444e11f 100644
    a b install_compiled_pkg() 
    6868for pkg in cohomolo-* crypting-* grape-* guava-* orb-*
    6969do
    7070    echo "Building GAP package $pkg"
     71    CFLAGS="$CFLAGS -Wno-implicit-function-declaration"
     72    export CFLAGS
    7173    cd "$PKG_SRC_DIR/$pkg"
    7274    ./configure "$GAP_ROOT"
    7375    sdh_make -j1

yes, I'm testing essentially the same branch, will post it soon.

comment:19 Changed 12 months ago by dimpase

  • Authors set to Dima Pasechnik, Matthias Koeppe
  • Branch changed from u/mkoeppe/fix_gap_packages_for_xcode_12 to u/dimpase/fix_gap_packages_for_xcode_12
  • Commit changed from 8693dc75891529b7e7e349c8ac4e1ac4076ea785 to 46e868eb824b4d6bb40b763fc7df186504df4aad
  • Status changed from new to needs_review

OK, this should be it

comment:20 Changed 12 months ago by mkoeppe

  • Reviewers changed from https://github.com/mkoeppe/sage/actions/runs/289319582 to https://github.com/mkoeppe/sage/actions/runs/296478189

comment:21 Changed 12 months ago by jhpalmieri

This isn't working for me. First, one of the patches is wrong:

  • build/pkgs/gap_packages/patches/cohomolo_makefile.patch

    diff --git a/build/pkgs/gap_packages/patches/cohomolo_makefile.patch b/build/pkgs/gap_packages/patches/cohomolo_makefile.patch
    index 401deba09d..01f0e85587 100644
    a b  
    1 diff --git a/Makefile.in b/Makefile.in
     1diff --git a/pkg/cohomolo-1.6.7/Makefile.in b/pkg/cohomolo-1.6.7/Makefile.in
    22index 96a6c75..ab8b8c3 100644
    3 --- a/Makefile.in
    4 +++ b/Makefile.in
     3--- a/pkg/cohomolo-1.6.7/Makefile.in
     4+++ b/pkg/cohomolo-1.6.7/Makefile.in
    55@@ -2,13 +2,18 @@
    66 # all executables are put into the bin directory.
    77 BIN = bin/@GAPARCH@

Even after fixing that, I get an error building gap_packages:

gcc -c -O2 -g -Wno-implicit-function-declaration -Wno-return-type -Wno-dangling-else -Wno-unused-result pcd.c 
pcd.c:22:8: error: redefinition of 'conj' as different kind of symbol
       conj[NPT],conjinv[NPT],facord[MEXP],pinv[NPT/2],rel[2*MEXP],
       ^
pcd.c:22:8: note: unguarded header; consider using #ifdef guards or #pragma once
pcd.c:22:8: note: previous definition is here
1 error generated.

Using only the change in comment:17 worked, but I'm not getting this new branch to work. Is anyone else having better luck?

comment:22 Changed 12 months ago by dimpase

Please make a branch that works for you and post it here. We'll review it for other systems.

comment:23 Changed 12 months ago by jhpalmieri

I can't push a branch, but these are the changes:

  • build/pkgs/gap_packages/spkg-install.in

    diff --git a/build/pkgs/gap_packages/spkg-install.in b/build/pkgs/gap_packages/spkg-install.in
    index 10b444b5c6..56c444e11f 100644
    a b install_compiled_pkg() 
    6868for pkg in cohomolo-* crypting-* grape-* guava-* orb-*
    6969do
    7070    echo "Building GAP package $pkg"
     71    CFLAGS="$CFLAGS -Wno-implicit-function-declaration"
     72    export CFLAGS
    7173    cd "$PKG_SRC_DIR/$pkg"
    7274    ./configure "$GAP_ROOT"
    7375    sdh_make -j1
  • src/sage/tests/gap_packages.py

    diff --git a/src/sage/tests/gap_packages.py b/src/sage/tests/gap_packages.py
    index 340ceb8c29..cf9a68cec2 100644
    a b TESTS:: 
    66    sage: from sage.tests.gap_packages import all_installed_packages, test_packages
    77    sage: pkgs = all_installed_packages(ignore_dot_gap=True)
    88    sage: test_packages(pkgs, only_failures=True)    # optional - gap_packages
     9    ...
    910      Status   Package   GAP Output
    1011    +--------+---------+------------+
    1112

comment:24 Changed 12 months ago by dimpase

what is happening when you are trying to push?

comment:25 Changed 12 months ago by jhpalmieri

I get an error like some of the ones reported earlier today:

% git trac push 30729        
Pushing to Trac #30729...
Guessed remote branch: u/jhpalmieri/fix_gap_packages_for_xcode_12
Traceback (most recent call last):
  File "/usr/local/bin/git-trac", line 17, in <module>
    cmdline.launch()
  File "/usr/local/lib/python3.8/site-packages/git_trac/cmdline.py", line 247, in launch
    app.push(ticket_number, remote=args.remote, force=args.force)
  File "/usr/local/lib/python3.8/site-packages/git_trac/app.py", line 218, in push
    self.repo.push(remote, force)
  File "/usr/local/lib/python3.8/site-packages/git_trac/git_repository.py", line 196, in push
    self.git.echo.push('trac', refspec)
  File "/usr/local/lib/python3.8/site-packages/git_trac/git_interface.py", line 340, in meth
    return self.execute(git_cmd, *args, **kwds)
  File "/usr/local/lib/python3.8/site-packages/git_trac/git_interface.py", line 95, in execute
    result = self._interface._run(cmd, args, kwds, 
  File "/usr/local/lib/python3.8/site-packages/git_trac/git_interface.py", line 262, in _run
    raise GitError(result)
git_trac.git_error.GitError: git returned with non-zero exit code (1) when executing "git push trac HEAD:refs/heads/u/jhpalmieri/fix_gap_packages_for_xcode_12"
    STDERR: remote: error: insufficient permission for adding an object to repository database ./objects
    STDERR: remote: fatal: failed to write object
    STDERR: error: remote unpack failed: unpack-objects abnormal exit
    STDERR: To trac.sagemath.org:sage.git
    STDERR:  ! [remote rejected]       HEAD -> u/jhpalmieri/fix_gap_packages_for_xcode_12 (unpacker error)
    STDERR: error: failed to push some refs to 'git@trac.sagemath.org:sage.git'

(Same with with git trac push ... --force)

Last edited 12 months ago by jhpalmieri (previous) (diff)

comment:26 Changed 12 months ago by dimpase

could you try pushing now?

I've learned about group permissions and did

setfacl -m "default:group::rwx" objects

Strange that ACL utils were not installed on the box, so I don't know how it worked before I messed it up...

Last edited 12 months ago by dimpase (previous) (diff)

comment:27 Changed 12 months ago by jhpalmieri

  • Branch changed from u/dimpase/fix_gap_packages_for_xcode_12 to u/jhpalmieri/fix_gap_packages_for_xcode_12

comment:28 Changed 12 months ago by jhpalmieri

  • Commit changed from 46e868eb824b4d6bb40b763fc7df186504df4aad to d8f07f93630c5bac197f3b525a5cbb2ab3fa096e

Success! Thank you.


New commits:

d8f07f9trac 30729: allow gap_packages to build with Xcode 12 by passing

comment:29 Changed 12 months ago by dimpase

  • Authors changed from Dima Pasechnik, Matthias Koeppe to Matthias Koeppe
  • Reviewers changed from https://github.com/mkoeppe/sage/actions/runs/296478189 to Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/296478189
  • Status changed from needs_review to positive_review

ok, this works on Debian.

comment:30 Changed 12 months ago by mkoeppe

  • Reviewers changed from Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/296478189 to Dima Pasechnik

comment:31 Changed 12 months ago by mkoeppe

  • Authors changed from Matthias Koeppe to Matthias Koeppe, John Palmieri

comment:32 Changed 11 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:33 Changed 11 months ago by vbraun

  • Branch changed from u/jhpalmieri/fix_gap_packages_for_xcode_12 to d8f07f93630c5bac197f3b525a5cbb2ab3fa096e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.