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

spkg-configure.m4 for ppl #29454

Closed
orlitzky opened this issue Apr 3, 2020 · 54 comments
Closed

spkg-configure.m4 for ppl #29454

orlitzky opened this issue Apr 3, 2020 · 54 comments

Comments

@orlitzky
Copy link
Contributor

orlitzky commented Apr 3, 2020

This is a C++ library with no pkg-config file, but at least upstream provides an m4 macro to detect itself.

Upstream: Reported upstream. No feedback yet.

CC: @dimpase @embray @mkoeppe @kiwifb

Component: build: configure

Author: Michael Orlitzky

Branch: fa6e0d9

Reviewer: Isuru Fernando, Matthias Koeppe

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

@orlitzky orlitzky added this to the sage-9.1 milestone Apr 3, 2020
@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

Commit: d766c29

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

Author: Michael Orlitzky

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

Branch: u/mjo/ticket/29454

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

New commits:

5b23dc7Trac #29454: add AM_PPL_PATH from the ppl package to detect itself.
eac273eTrac #29454: new spkg-configure.m4 for ppl.
d766c29Trac #29454: add distros/* information for ppl.

@isuruf
Copy link
Member

isuruf commented Apr 3, 2020

comment:2

On conda, the package is ppl.

I get the following error,

## ---------------------------------------------------- ##
## Checking whether SageMath should install SPKG ppl... ##
## ---------------------------------------------------- ##
configure:24645: checking whether any of glpk gmp mpir is installed or will be installed as SPKG
configure:24654: result: no
configure:24706: checking for ppl-config
configure:24724: found /projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/ppl-config
configure:24737: result: /projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/ppl-config
configure:24751: checking for the Parma Polyhedra Library, version >= 1.2
configure:24912: /projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/x86_64-conda_cos6-linux-gnu-c++ -std=gnu++11 -o conftest -fvisibility-inlines-hidden -std=c++17 -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/include -I/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -I/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/include -I/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/include -Wl,-O2 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,--disable-new-dtags -Wl,--gc-sections -Wl,-rpath,/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/lib -Wl,-rpath-link,/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/lib -L/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/lib -lppl -lgmpxx -lgmp -Wl,-O2 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,--disable-new-dtags -Wl,--gc-sections -Wl,-rpath,/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/lib -Wl,-rpath-link,/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/lib -L/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/lib conftest.cpp -lmpc -lm4rie -llrcalc -lglpk -lgf2x -lecm -L/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/lib -lcurl -lcliquer -lbz2 -larb -lflint -lmpfr -lgmp -lm  -lntl -lec -lLfunction >&5
conftest.cpp: In function 'int main()':
conftest.cpp:78:9: warning: ignoring return value of 'int system(const char*)', declared with attribute warn_unused_result [-Wunused-result]
   system("touch conf.ppltest");
   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.3.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: /tmp/ccTIhRI4.o: in function `main':
conftest.cpp:(.text.startup.main+0xd0): undefined reference to `Parma_Polyhedra_Library::version()'
/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.3.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: conftest.cpp:(.text.startup.main+0xea): undefined reference to `Parma_Polyhedra_Library::version()'
/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.3.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: conftest.cpp:(.text.startup.main+0x169): undefined reference to `Parma_Polyhedra_Library::version()'
/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.3.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: conftest.cpp:(.text.startup.main+0x273): undefined reference to `Parma_Polyhedra_Library::version()'
/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.3.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: /tmp/ccTIhRI4.o: in function `_GLOBAL__sub_I_conftest.cpp':
conftest.cpp:(.text.startup._GLOBAL__sub_I_conftest.cpp+0x35): undefined reference to `Parma_Polyhedra_Library::Init::Init()'
/projects/66d93023-00f0-4c12-8a25-5d6d4e486740/sage-build/bin/../lib/gcc/x86_64-conda_cos6-linux-gnu/7.3.0/../../../../x86_64-conda_cos6-linux-gnu/bin/ld: conftest.cpp:(.text.startup._GLOBAL__sub_I_conftest.cpp+0x3c): undefined reference to `Parma_Polyhedra_Library::Init::~Init()'
collect2: error: ld returned 1 exit status
configure:24912: $? = 1
configure: program exited with status 1

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:3

Hm... it's passing -lppl to the link command, is your conda library path on that command-line somewhere?

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 3, 2020

Changed branch from u/mjo/ticket/29454 to u/mkoeppe/ticket/29454

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 3, 2020

comment:5

You can test conda using #29417:
tox -e local-conda-forge-standard


New commits:

3f7f037build/pkgs/ppl/distros/debian.txt: fixup

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 3, 2020

Changed commit from d766c29 to 3f7f037

@isuruf
Copy link
Member

isuruf commented Apr 3, 2020

comment:6

Looks like -Wl,--as-needed -lppl conftest.cpp doesn't work. -Wl,--as-needed is added by conda. -lppl needs to come after conftest.cpp.


New commits:

3f7f037build/pkgs/ppl/distros/debian.txt: fixup

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:7

Something wrong with their m4 macro, then? They stick the -lppl in LDFLAGS instead of LIBS.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 3, 2020

comment:8

Yes, that's wrong

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:9

See if this works? I've hacked their macro to loop through the output of ppl-config --ldflags, putting each flag into either PPL_LDFLAGS or PPL_LIBS based on whether or not the flag starts with -l. This feels reeeaaally sketchy, but it seems to do the right thing on my machine. If it's semi-reliable, I can suggest it to upstream.


New commits:

23d2b36Trac #29454: hack m4/ppl.m4 to define PPL_LIBS.
f9f18bdbuild/pkgs/ppl/distros/debian.txt: fixup

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

Changed branch from u/mkoeppe/ticket/29454 to u/mjo/ticket/29454

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

Changed commit from 3f7f037 to f9f18bd

@isuruf
Copy link
Member

isuruf commented Apr 3, 2020

comment:10

Works for me. Can you add ppl for conda as well?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 3, 2020

Changed commit from f9f18bd to c172077

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 3, 2020

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

c172077Trac #29454: add conda package information for PPL.

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:12

Yeah, sorry, I forgot.

@isuruf
Copy link
Member

isuruf commented Apr 3, 2020

Reviewer: Isuru Fernando

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 3, 2020

comment:14

Upstream report for posterity: https://www.cs.unipr.it/mantis/view.php?id=2638

@dimpase
Copy link
Member

dimpase commented Apr 4, 2020

Upstream: Reported upstream. No feedback yet.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 4, 2020

comment:25

See https://src.fedoraproject.org/rpms/ppl/blob/master/f/ppl.spec on how Fedora builds its ppl packages.

In particular:

# In order to avoid multiarch conflicts when installed for multiple
# architectures (e.g., i386 and x86_64), we rename the header files
# of the ppl-devel package.  They are substituted with ad-hoc 
# switchers that select the appropriate header file depending on
# the architecture for which the compiler is compiling.
 
# Since our header files only depend on the sizeof things, we smash
# ix86 onto i386 and arm* onto arm.  For the SuperH RISC engine family,
# we smash sh3 and sh4 onto sh.
normalized_arch=%{_arch}
%ifarch %{ix86}
normalized_arch=i386
%endif
%ifarch %{arm}
normalized_arch=arm
%endif
%ifarch sh3 sh4
normalized_arch=sh
%endif
 
mv %{buildroot}/%{_includedir}/ppl.hh %{buildroot}/%{_includedir}/ppl-${normalized_arch}.hh
install -m644 %{SOURCE1} %{buildroot}/%{_includedir}/ppl.hh
mv %{buildroot}/%{_includedir}/ppl_c.h %{buildroot}/%{_includedir}/ppl_c-${normalized_arch}.h
install -m644 %{SOURCE2} %{buildroot}/%{_includedir}/ppl_c.h

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 4, 2020

comment:26
install -m644 %{SOURCE1} %{buildroot}/%{_includedir}/ppl.hh

That should be installing their copy of ppl.hh that "selects the appropriate header file depending on the architecture for which the compiler is compiling."

@dimpase
Copy link
Member

dimpase commented Apr 4, 2020

comment:27

Fedora 30 has packages ppl and ppl-devel, and ppl-config in one of them.

$ ppl-config -i
/usr/include

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 4, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ec3a771Trac #29454: require ppl-dev(el) on Debian/Fedora.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 4, 2020

Changed commit from 7c69b90 to ec3a771

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 4, 2020

comment:29

Thanks, I force-pushed over that last commit to add ppl-devel on Fedora. Maybe that solves the header problem.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 4, 2020

comment:30

Testing at https://github.com/mkoeppe/sage/actions/runs/70615046

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 5, 2020

comment:31

fedora needs more work, see for example https://github.com/mkoeppe/sage/suites/572702630/artifacts/3888966

configure:26405: checking whether any of glpk gmp mpir is installed as or will be installed as SPKG
configure:26414: result: no
configure:26469: checking for ppl-config
configure:26487: found /usr/bin/ppl-config
configure:26500: result: /usr/bin/ppl-config
configure:26514: checking for the Parma Polyhedra Library, version >= 1.2
configure:26685: g++ -std=gnu++11 -o conftest -g -O2   -I/usr/lib64/swipl-8.0.2/include -I/usr/lib64/gprolog-1.4.5/include  -L/usr/lib64 -Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld  conftest.cpp -lm4rie -llrcalc -lglpk -lgf2x -lecm -lpari -lcurl -lcliquer -lbz2 -lgmp -lm  -lntl -lec  -lppl -lgmpxx -lgmp >&5
/usr/bin/ld: /tmp/ccgB763W.o: relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a PIE object; recompile with -fPIC
/usr/bin/ld: final link failed: nonrepresentable section on output
collect2: error: ld returned 1 exit status
configure:26685: $? = 1
configure: program exited with status 1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 5, 2020

Changed commit from ec3a771 to fa6e0d9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 5, 2020

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

fa6e0d9Trac #29454: don't use PPL's LDFLAGS to compile its test program.

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 5, 2020

comment:33

One more try! That's another thing I'll have to report upstream if the new commit fixes the issue.

@orlitzky
Copy link
Contributor Author

orlitzky commented Apr 5, 2020

comment:34

Thanks for setting up this test infrastructure, by the way. I occasionally feel personally attacked by github, but it's ultimately better to find and fix these problems early on.

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 6, 2020

comment:35

Looking good now. (Tests at https://github.com/mkoeppe/sage/actions/runs/71360666)

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 6, 2020

Changed reviewer from Isuru Fernando to Isuru Fernando, Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented Apr 9, 2020

Changed branch from u/mjo/ticket/29454 to fa6e0d9

@dimpase
Copy link
Member

dimpase commented Apr 13, 2020

Changed commit from fa6e0d9 to none

@dimpase
Copy link
Member

dimpase commented Apr 13, 2020

comment:37

there is a (hopefully innocent) no-op

+  [ # Some of its dependencies are installed as SPKGs, so install the
+    # ppl SPKG as well.
+    sage_spkg_install_ppl=yes

there is actually no need to set this var to yes, anyway, as it's yes before the macro is run

@jhpalmieri
Copy link
Member

comment:38

Regarding homebrew: this suggests using homebrew to install ppl, but Sage then doesn't allow that:

configure:27049: checking whether any of glpk gmp mpir is installed as or will be installed as SPKG
configure:27053: result: yes; install ppl as well
configure:27448: no suitable system package found for SPKG ppl

And this is because Sage doesn't allow glpk: the file build/pkgs/glpk/distros/homebrew.txt says

## glpk 4.65 should not be used by Sage as long as it is not patched by Homebrew
# glpk

Is this because of the doctest failures which have been fixed at #29317? That is, can we use an unpatched glpk? Should I open up a ticket to allow homebrew's glpk?

@orlitzky
Copy link
Contributor Author

comment:39

Replying to @dimpase:

there is a (hopefully innocent) no-op

+  [ # Some of its dependencies are installed as SPKGs, so install the
+    # ppl SPKG as well.
+    sage_spkg_install_ppl=yes

there is actually no need to set this var to yes, anyway, as it's yes before the macro is run

I've made this (apparently harmless in all cases) mistake a few times by copy/pasting something that never worked to begin with. I left myself a note to go back and delete them all.

@dimpase
Copy link
Member

dimpase commented Apr 26, 2020

comment:40

Replying to @jhpalmieri:

Regarding homebrew: this suggests using homebrew to install ppl, but Sage then doesn't allow that:

configure:27049: checking whether any of glpk gmp mpir is installed as or will be installed as SPKG
configure:27053: result: yes; install ppl as well
configure:27448: no suitable system package found for SPKG ppl

And this is because Sage doesn't allow glpk: the file build/pkgs/glpk/distros/homebrew.txt says

## glpk 4.65 should not be used by Sage as long as it is not patched by Homebrew
# glpk

Is this because of the doctest failures which have been fixed at #29317? That is, can we use an unpatched glpk? Should I open up a ticket to allow homebrew's glpk?

We do use unpatched (to some extent) glpk on various Linux distos, I don't see why Homebrew is special in this sense. By the way, did you try submitting a Homebrew issue to fix this? I recently managed to talk them into fixing an annoying thing about their gettext package.

Nothing prevents you from installing glkp on Homebrew and using it regardless,
it's merely a hint that it's not printed (and thus not picked by the AI that does automated installs of Sage on Github Actions CI).

@jhpalmieri
Copy link
Member

comment:41

I see this when I use the system's ppl:

sage -t --long --warn-long 80.9 src/sage/misc/package.py
**********************************************************************
File "src/sage/misc/package.py", line 200, in sage.misc.package.list_packages
Failed example:
    L['ppl']  # optional - build
Expected:
    {'installed': True,
     'installed_version': '...',
     'remote_version': '...',
     'type': 'standard'}
Got:
    {'installed': False,
     'installed_version': None,
     'name': 'ppl',
     'remote_version': '1.2.p1',
     'source': 'normal',
     'type': 'standard'}
**********************************************************************

@dimpase
Copy link
Member

dimpase commented Apr 26, 2020

comment:42

probably noone tested with # optional - build on, so it slipped through the cracks.
Obviously, a system package is not "installed" - in the sense, not by Sage.
The doctest has to be changed, to another package we don't have from a system (so this is a shrining list)...

@jhpalmieri
Copy link
Member

comment:43

I've opened #29591.

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

6 participants