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

Let cddlib build a shared lib on Cygwin. #15872

Closed
jpflori opened this issue Feb 27, 2014 · 16 comments
Closed

Let cddlib build a shared lib on Cygwin. #15872

jpflori opened this issue Feb 27, 2014 · 16 comments

Comments

@jpflori
Copy link

jpflori commented Feb 27, 2014

Follow up to #13354 and #13026.

Upstream PR: cddlib/cddlib#9

Depends on #21952

Upstream: Completely fixed; Fix reported upstream

Component: porting: Cygwin

Keywords: cddlib windows cygwin

Reviewer: Travis Scrimshaw

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

@jpflori jpflori added this to the sage-6.2 milestone Feb 27, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@embray
Copy link
Contributor

embray commented Nov 23, 2016

comment:3

Interesting that there's already a ticket for this. I've basically done the work needed for this, but a mere patch won't suffice--I think the cddlib source package needs to be rebuilt to have any hope at resolving this. See https://groups.google.com/d/msg/sage-devel/uHsTOd5sTxY/ivpNHT1MAwAJ

@embray embray removed this from the sage-6.4 milestone Nov 23, 2016
@embray embray self-assigned this Nov 23, 2016
@embray
Copy link
Contributor

embray commented Nov 23, 2016

Changed keywords from none to cddlib windows cygwin

@embray
Copy link
Contributor

embray commented Nov 23, 2016

comment:5

FWIW these are the basic changes that are needed:

diff -u a/configure.in b/configure.in
--- a/configure.in      2016-11-23 11:40:16.064457900 +0100
+++ b/configure.in      2016-11-22 16:39:29.311747000 +0100
@@ -7,7 +7,18 @@
 dnl Checks for programs.
 AC_PROG_CC
 AC_PROG_INSTALL
-LT_INIT
+LT_INIT([win32-dll])
+
+AC_CANONICAL_HOST
+dnl libtool requires "-no-undefined" for win32 dll
+AC_SUBST(CDD_LDFLAGS)
+case $host_os in
+  *cygwin* | *mingw*)
+    if test x"$enable_shared" = xyes; then
+      CDD_LDFLAGS="$CDD_LDFLAGS -no-undefined"
+    fi
+;;
+esac

 dnl Checks for libraries.
 dnl Replace `main' with a function in -lg:
diff -u a/lib-src/Makefile.am b/lib-src/Makefile.am
--- a/lib-src/Makefile.am       2016-11-23 11:40:40.796141400 +0100
+++ b/lib-src/Makefile.am       2016-11-23 11:43:44.183481400 +0100
@@ -10,6 +10,8 @@
 setoper.c \
 random.c

+libcdd_la_LDFLAGS = $(CDD_LDFLAGS)
+
 include_HEADERS = \
 cdd.h \
 cddmp.h \
diff -u a/lib-src-gmp/Makefile.am b/lib-src-gmp/Makefile.am
--- a/lib-src-gmp/Makefile.am   2016-11-23 11:40:45.065312300 +0100
+++ b/lib-src-gmp/Makefile.am   2016-11-17 17:00:16.713039400 +0100
@@ -16,6 +16,8 @@
 setoper.c \
 random.c

+libcddgmp_la_LDFLAGS = $(CDD_LDFLAGS)
+
 include_HEADERS = \
 cdd.h \
 cddmp.h \

@embray
Copy link
Contributor

embray commented Nov 23, 2016

Dependencies: #21952

@embray
Copy link
Contributor

embray commented Nov 23, 2016

comment:6

Adding the changes needed for building shared libs on Cygwin is going to require #21952.

@saraedum
Copy link
Member

Upstream: Not yet reported upstream; Will do shortly.

@saraedum
Copy link
Member

comment:7

If you wouldn't mind, could you propose these changes as an issue at https://github.com/cddlib/cddlib?

@saraedum
Copy link
Member

comment:8

I created cddlib/cddlib#8 to track this issue upstream.

@embray
Copy link
Contributor

embray commented May 14, 2018

Changed upstream from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.

@embray

This comment has been minimized.

@saraedum
Copy link
Member

saraedum commented Jul 4, 2018

comment:10

Can this be closed now?

@embray
Copy link
Contributor

embray commented Jul 10, 2018

comment:11

I wouldn't until/unless #25344 is closed/fixed.

@mkoeppe
Copy link
Member

mkoeppe commented Sep 22, 2018

comment:12

#25344 is in.

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Sep 22, 2018

Changed upstream from Reported upstream. No feedback yet. to Completely fixed; Fix reported upstream

@embray
Copy link
Contributor

embray commented Sep 24, 2018

comment:15

Just to confirm, this does appear to (still) have worked with #25344.

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

5 participants