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

Fix gap_packages for Xcode 12 #30729

Closed
mkoeppe opened this issue Oct 5, 2020 · 41 comments
Closed

Fix gap_packages for Xcode 12 #30729

mkoeppe opened this issue Oct 5, 2020 · 41 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 5, 2020

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.

Depends on #30720

Upstream: Reported upstream. No feedback yet.

CC: @dimpase @jhpalmieri @soehms

Component: packages: optional

Author: Matthias Koeppe, John Palmieri

Branch/Commit: d8f07f9

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-9.2 milestone Oct 5, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2020

Dependencies: #30720

@dimpase
Copy link
Member

dimpase commented Oct 5, 2020

comment:2

I've opened gap-packages/cohomolo#27

@dimpase
Copy link
Member

dimpase commented Oct 5, 2020

Upstream: Reported upstream. No feedback yet.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2020

comment:3

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

@dimpase
Copy link
Member

dimpase commented Oct 5, 2020

comment:4

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

@jhpalmieri
Copy link
Member

comment:5

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 5, 2020

comment:6

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 6, 2020

comment:7

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 6, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 6, 2020

comment:9

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

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 6, 2020

@jhpalmieri
Copy link
Member

Commit: 8693dc7

@jhpalmieri
Copy link
Member

comment:11

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

@jhpalmieri

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Oct 7, 2020

comment:12

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).

@dimpase
Copy link
Member

dimpase commented Oct 8, 2020

comment:13

Even quicker should be to do

diff --git a/build/pkgs/gap_packages/spkg-install.in b/build/pkgs/gap_packages/spkg-install.in
index 10b444b5c6..cc094ad40c 100644
--- a/build/pkgs/gap_packages/spkg-install.in
+++ b/build/pkgs/gap_packages/spkg-install.in
@@ -3,6 +3,8 @@ PKG_DIR="$GAP_ROOT/pkg"
 
 PKG_SRC_DIR="$(pwd)/src/pkg"
 cd "$PKG_SRC_DIR"
+CFLAGS = "$CFLAGS -Wno-implicit-function-declaration"
+export CFLAGS
 
 # directly install pure GAP packages
 #

@jhpalmieri
Copy link
Member

comment:14

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

@dimpase
Copy link
Member

dimpase commented Oct 8, 2020

comment:15

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?

@dimpase
Copy link
Member

dimpase commented Oct 8, 2020

comment:16

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

@jhpalmieri
Copy link
Member

comment:17

This works for me, instead:

diff --git a/build/pkgs/gap_packages/spkg-install.in b/build/pkgs/gap_packages/spkg-install.in
index 10b444b5c6..56c444e11f 100644
--- a/build/pkgs/gap_packages/spkg-install.in
+++ b/build/pkgs/gap_packages/spkg-install.in
@@ -68,6 +68,8 @@ install_compiled_pkg()
 for pkg in cohomolo-* crypting-* grape-* guava-* orb-*
 do
     echo "Building GAP package $pkg"
+    CFLAGS="$CFLAGS -Wno-implicit-function-declaration"
+    export CFLAGS
     cd "$PKG_SRC_DIR/$pkg"
     ./configure "$GAP_ROOT"
     sdh_make -j1

@dimpase
Copy link
Member

dimpase commented Oct 8, 2020

comment:18

Replying to @jhpalmieri:

This works for me, instead:

diff --git a/build/pkgs/gap_packages/spkg-install.in b/build/pkgs/gap_packages/spkg-install.in
index 10b444b5c6..56c444e11f 100644
--- a/build/pkgs/gap_packages/spkg-install.in
+++ b/build/pkgs/gap_packages/spkg-install.in
@@ -68,6 +68,8 @@ install_compiled_pkg()
 for pkg in cohomolo-* crypting-* grape-* guava-* orb-*
 do
     echo "Building GAP package $pkg"
+    CFLAGS="$CFLAGS -Wno-implicit-function-declaration"
+    export CFLAGS
     cd "$PKG_SRC_DIR/$pkg"
     ./configure "$GAP_ROOT"
     sdh_make -j1

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

@dimpase
Copy link
Member

dimpase commented Oct 8, 2020

@dimpase
Copy link
Member

dimpase commented Oct 8, 2020

Author: Dima Pasechnik, Matthias Koeppe

@dimpase
Copy link
Member

dimpase commented Oct 8, 2020

Changed commit from 8693dc7 to 46e868e

@dimpase
Copy link
Member

dimpase commented Oct 8, 2020

comment:19

OK, this should be it

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 8, 2020

@jhpalmieri
Copy link
Member

comment:21

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

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/build/pkgs/gap_packages/patches/cohomolo_makefile.patch
+++ b/build/pkgs/gap_packages/patches/cohomolo_makefile.patch
@@ -1,7 +1,7 @@
-diff --git a/Makefile.in b/Makefile.in
+diff --git a/pkg/cohomolo-1.6.7/Makefile.in b/pkg/cohomolo-1.6.7/Makefile.in
 index 96a6c75..ab8b8c3 100644
---- a/Makefile.in
-+++ b/Makefile.in
+--- a/pkg/cohomolo-1.6.7/Makefile.in
++++ b/pkg/cohomolo-1.6.7/Makefile.in
 @@ -2,13 +2,18 @@
  # all executables are put into the bin directory.
  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?

@dimpase
Copy link
Member

dimpase commented Oct 9, 2020

comment:22

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

@jhpalmieri
Copy link
Member

comment:23

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

diff --git a/build/pkgs/gap_packages/spkg-install.in b/build/pkgs/gap_packages/spkg-install.in
index 10b444b5c6..56c444e11f 100644
--- a/build/pkgs/gap_packages/spkg-install.in
+++ b/build/pkgs/gap_packages/spkg-install.in
@@ -68,6 +68,8 @@ install_compiled_pkg()
 for pkg in cohomolo-* crypting-* grape-* guava-* orb-*
 do
     echo "Building GAP package $pkg"
+    CFLAGS="$CFLAGS -Wno-implicit-function-declaration"
+    export CFLAGS
     cd "$PKG_SRC_DIR/$pkg"
     ./configure "$GAP_ROOT"
     sdh_make -j1
diff --git a/src/sage/tests/gap_packages.py b/src/sage/tests/gap_packages.py
index 340ceb8c29..cf9a68cec2 100644
--- a/src/sage/tests/gap_packages.py
+++ b/src/sage/tests/gap_packages.py
@@ -6,6 +6,7 @@ TESTS::
     sage: from sage.tests.gap_packages import all_installed_packages, test_packages
     sage: pkgs = all_installed_packages(ignore_dot_gap=True)
     sage: test_packages(pkgs, only_failures=True)    # optional - gap_packages
+    ...
       Status   Package   GAP Output
     +--------+---------+------------+
 

@dimpase
Copy link
Member

dimpase commented Oct 9, 2020

comment:24

what is happening when you are trying to push?

@jhpalmieri
Copy link
Member

comment:25

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 '[email protected]:sage.git'

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

@dimpase
Copy link
Member

dimpase commented Oct 9, 2020

comment:26

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...

@jhpalmieri
Copy link
Member

@jhpalmieri
Copy link
Member

comment:28

Success! Thank you.


New commits:

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

@jhpalmieri
Copy link
Member

Changed commit from 46e868e to d8f07f9

@dimpase
Copy link
Member

dimpase commented Oct 9, 2020

Changed author from Dima Pasechnik, Matthias Koeppe to Matthias Koeppe

@dimpase
Copy link
Member

dimpase commented Oct 9, 2020

comment:29

ok, this works on Debian.

@dimpase
Copy link
Member

dimpase commented Oct 9, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 10, 2020

Changed reviewer from Dima Pasechnik, https://github.com/mkoeppe/sage/actions/runs/296478189 to Dima Pasechnik

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 10, 2020

Changed author from Matthias Koeppe to Matthias Koeppe, John Palmieri

@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Oct 24, 2020
@vbraun
Copy link
Member

vbraun commented Oct 31, 2020

Changed branch from u/jhpalmieri/fix_gap_packages_for_xcode_12 to d8f07f9

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