Let libgap build a shared library on Cygwin

[jpflori]

We need to pass -no-undefined to libtool, the proper place is in libgap_la_LDFLAGS, potentially only on Cygwin and other Windows systems (have a look in GMP/MPIR/MPFR for examples), but should not hurt anyway elsewhere if added unconditionally.

(By the way the spkg-install script is terrible, does not use $MAKE, use [ ] and [[ ]], sets CXXFLAGS but does not export it...)

http://www.stp.dias.ie/~vbraun/Sage/spkg/libgap-4.5.7.p1.spkg

CC: @kcrisman @dimpase @vbraun

Component: packages: standard

Keywords: cygwin spkg libgap

Author: Volker Braun, Jean-Pierre Flori

Reviewer: Jean-Pierre Flori, Dmitrii Pasechnik

Merged: sage-5.8.beta0

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

[jpflori]

comment:1

And now MPIR defines __GNU_MP_RELEASE so the __GMP_MP_RELEASE hack is not needed anymore here or in gap spkg.

[vbraun]

comment:2

I made #14039 to keep track of what to change for the next GAP update.

[vbraun]

comment:3

I've made a new spkg that includes the --no-undefined. Please let me know if it fixes things.

[vbraun]

Author: Volker Braun

[jpflori]

comment:4

I've had a quick look at the spkg and it seems fine except there should be only one dash in front of no-undefined (not sure though putting two will break things).

The spkg-install cript looks cleaner.
Would you mind replacing the last double brackets by simple ones?
The use of double brackets does not seem necerssary for the simple tests involved.

A line for the version bump in SPKG.txt would be nice as well.

It should fix things on Cygwin (I crafted my own spkg yesterday and everything went fine, see CygwinPort).
The question should then rather be: Does it break things on other systems? (it should not.)

[jpflori]

Reviewer: Jean-Pierre Flori

[vbraun]

comment:5

Long-style is --no-undefined (two dashes), short is -z. ld is being nice and also accepts the mongrel -no-undefined but we shouldn't rely on that imho.

I made the other cosmetic changes.

It doesn't break anything on Linux. After you review the spkg, the buildbot will catch anything that breaks on exotic platforms.

[jpflori]

comment:7

Replying to @vbraun:

Long-style is --no-undefined (two dashes), short is -z. ld is being nice and also accepts the mongrel -no-undefined but we shouldn't rely on that imho.

Its not an ld flag its a libtool one and its definitely one dash.
The fact that libtool does not follow usual conventions is another problem.

I made the other cosmetic changes.

It doesn't break anything on Linux. After you review the spkg, the buildbot will catch anything that breaks on exotic platforms.

[jpflori]

comment:8

(And I seem to remember there should be a LIBTOOLFLAGS var or stg like that, but I don't think anyone uses is, not even sure libtool uses it itself...)

[dimpase]

comment:9

Why do we even need CXXFLAGS? There is no C++ in GAP, isn't it?

[dimpase]

comment:10

Please see https://bitbucket.org/vbraun/libgap/pull-requests/1/cf-sage-trac-14038/diff
for updates on this

[jpflori]

comment:11

Replying to @dimpase:

Why do we even need CXXFLAGS? There is no C++ in GAP, isn't it?

Don't know but I feel you're right.
Id say its an artifact from the time where spkg-install files' headers were all formatted the same.

[dimpase]

comment:12

Replying to @dimpase:

Please see https://bitbucket.org/vbraun/libgap/pull-requests/1/cf-sage-trac-14038/diff
for updates on this

so we need a Python patch for this to work on OSX.
http://hg.python.org/cpython/rev/864b9836dae6
I've opened #14057 for this.

[dimpase]

Dependencies: #14057

[dimpase]

comment:14

Even after applying #14057 the modified libgap does not build on OSX 10.6.8.
Does it need more libtool magic? I get:

libtool: link: gcc -dynamiclib  -o .libs/libgap.0.dylib  .libs/libgap_la-ariths.o .libs/libgap_la-c_random.o .libs/libgap_la-gmpints.o .libs/libgap_la-objccoll.o .libs/libgap_la-rational.o .libs/libgap_la-system.o .libs/libgap_la-blister.o .libs/libgap_la-c_type1.o .libs/libgap_la-gvars.o .libs/libgap_la-objcftl.o .libs/libgap_la-read.o .libs/libgap_la-tietze.o .libs/libgap_la-bool.o .libs/libgap_la-cyclotom.o .libs/libgap_la-integer.o .libs/libgap_la-objects.o .libs/libgap_la-records.o .libs/libgap_la-vars.o .libs/libgap_la-calls.o .libs/libgap_la-dt.o .libs/libgap_la-intfuncs.o .libs/libgap_la-objfgelm.o .libs/libgap_la-saveload.o .libs/libgap_la-vec8bit.o .libs/libgap_la-c_filt1.o .libs/libgap_la-dteval.o .libs/libgap_la-intrprtr.o .libs/libgap_la-objpcgel.o .libs/libgap_la-scanner.o .libs/libgap_la-vecffe.o .libs/libgap_la-c_meths1.o .libs/libgap_la-exprs.o .libs/libgap_la-iostream.o .libs/libgap_la-objscoll.o .libs/libgap_la-sctable.o .libs/libgap_la-vecgf2.o .libs/libgap_la-code.o .libs/libgap_la-finfield.o .libs/libgap_la-libgap.o .libs/libgap_la-opers.o .libs/libgap_la-set.o .libs/libgap_la-vector.o .libs/libgap_la-compiler.o .libs/libgap_la-funcs.o .libs/libgap_la-listfunc.o .libs/libgap_la-permutat.o .libs/libgap_la-stats.o .libs/libgap_la-weakptr.o .libs/libgap_la-compstat.o .libs/libgap_la-gap.o .libs/libgap_la-listoper.o .libs/libgap_la-plist.o .libs/libgap_la-streams.o .libs/libgap_la-c_oper1.o .libs/libgap_la-lists.o .libs/libgap_la-precord.o .libs/libgap_la-string.o .libs/libgap_la-costab.o .libs/libgap_la-gasman.o .libs/libgap_la-macfloat.o .libs/libgap_la-range.o .libs/libgap_la-sysfiles.o   -L/usr/local/src/sage/sage-5.6.beta2/local/lib /usr/local/src/sage/sage-5.6.beta2/local/lib/libgmp.dylib    -install_name  /usr/local/src/sage/sage-5.6.beta2/local/lib/libgap.0.dylib -compatibility_version 1 -current_version 1.0 -Wl,-single_module
Undefined symbols:
  "_environ", referenced from:
      _libgap_initialize in libgap_la-libgap.o
      _libGAP_SyExecuteProcess in libgap_la-sysfiles.o
ld: symbol(s) not found
collect2: ld returned 1 exit status
make[2]: *** [libgap.la] Error 1

[dimpase]

comment:15

Replying to @dimpase:

Even after applying #14057 the modified libgap does not build on OSX 10.6.8.
Does it need more libtool magic?

Specifying -no-undefined only for CYGWIN will fix this. I don't know how to do this properly, though. Any tips?

[jpflori]

comment:16

Yup Ill post a patch.
First time I see -no-undefined break something. Nice!

[jpflori]

comment:22

Could you post a log of a succesful build of libgap on your OS X somewhere?
And a failed one when no-undefined is passed to libtool?
I'd be interested in seeing what happens.

[jpflori]

comment:23

Whatever, I guess that passing -no-undefined to libtool just overrides its default behavior to pass -undefined dynamic_lookup to ld on OS X.

So we have two solutions:

• only pass -no-undefined where it is really needed, that is on Cygwin,
• use _NSGetEnviron() routine from <crt_externs.h> to define environ on OS X.

[dimpase]

comment:24

Replying to @jpflori:

Whatever, I guess that passing -no-undefined to libtool just overrides its default behavior to pass -undefined dynamic_lookup to ld on OS X.

indeed.

So we have two solutions:

• only pass -no-undefined where it is really needed, that is on Cygwin,
• use _NSGetEnviron() routine from <crt_externs.h> to define environ on OS X.

let us try to see if the latter is needed. My understanding is that currently the corresponding part of GAP functionality (ExecuteProcess()) is not exposed, and/or not implemented, in libGAP, and so it's not visible whether it is a bug to use -undefined dynamic_lookup to get (or not?) environ on OSX.

Or, perhaps, Volker knows the answer already? Perhaps he's not going to use this part of the orginal GAP code in libGAP at all?

[dimpase]

comment:25

Replying to @dimpase:

Replying to @jpflori:

Whatever, I guess that passing -no-undefined to libtool just overrides its default behavior to pass -undefined dynamic_lookup to ld on OS X.

indeed.

So we have two solutions:

• only pass -no-undefined where it is really needed, that is on Cygwin,
• use _NSGetEnviron() routine from <crt_externs.h> to define environ on OS X.

let us try to see if the latter is needed. My understanding is that currently the corresponding part of GAP functionality (ExecuteProcess()) is not exposed, and/or not implemented, in libGAP, and so it's not visible whether it is a bug to use -undefined dynamic_lookup to get (or not?) environ on OSX.

My understanding from e.g. this thread is that extern char** environ with -no-undefined for a shared library used with the OSX ld and _NSGetEnviron() lead to equivalent results.

So it seems that to make this change non-OSX only, or Cygwin-only, is right. AFAIK, Volker is currently away, so you can have a go at it yourself if you have time.

[jpflori]

comment:26

I don't really follow you here.
From what you linked, it seems that using an extern environ won't work (unless we allow undefined vars at compile time and hope that at runtime something will have properly defined and exported environ and we can link to it, which I find bad practice), and that instead one should use _NSGetEnviron() (which is available and can be linked in at compile time).

Even though only defining -no-undefined on Cygwin can only smoothen things, I don't think it would be a mistake to define it unconditionally, provided we correctly define environ in all situations.

[jpflori]

comment:27

Could you try the spkg at
http://boxen.math.washington.edu/home/jpflori/libgap-4.5.7.p1.spkg
?

Just to see if using _NSGetEnviron is enough to solve our problem on OS X.

[dimpase]

comment:28

Replying to @jpflori:

I don't really follow you here.
From what you linked, it seems that using an extern environ won't work (unless we allow undefined vars at compile time and hope that at runtime something will have properly defined and exported environ and we can link to it, which I find bad practice),

Well, that seems to be the usual practice on OSX, to delay such resolutions to ld. If you read
the last message in the tread I mentioned, that's exactly what it says.

I just tried your new spkg, and you still have at least one place (in src/src/sysfiles.c, I guess) where environ is undefined:

libtool: link: gcc -dynamiclib  -o .libs/libgap.0.dylib  .libs/libgap_la-ariths.o .libs/libgap_la-c_random.o .libs/libgap_la-gmpints.o .libs/libgap_la-objccoll.o .libs/libgap_la-rational.o .libs/libgap_la-system.o .libs/libgap_la-blister.o .libs/libgap_la-c_type1.o .libs/libgap_la-gvars.o .libs/libgap_la-objcftl.o .libs/libgap_la-read.o .libs/libgap_la-tietze.o .libs/libgap_la-bool.o .libs/libgap_la-cyclotom.o .libs/libgap_la-integer.o .libs/libgap_la-objects.o .libs/libgap_la-records.o .libs/libgap_la-vars.o .libs/libgap_la-calls.o .libs/libgap_la-dt.o .libs/libgap_la-intfuncs.o .libs/libgap_la-objfgelm.o .libs/libgap_la-saveload.o .libs/libgap_la-vec8bit.o .libs/libgap_la-c_filt1.o .libs/libgap_la-dteval.o .libs/libgap_la-intrprtr.o .libs/libgap_la-objpcgel.o .libs/libgap_la-scanner.o .libs/libgap_la-vecffe.o .libs/libgap_la-c_meths1.o .libs/libgap_la-exprs.o .libs/libgap_la-iostream.o .libs/libgap_la-objscoll.o .libs/libgap_la-sctable.o .libs/libgap_la-vecgf2.o .libs/libgap_la-code.o .libs/libgap_la-finfield.o .libs/libgap_la-libgap.o .libs/libgap_la-opers.o .libs/libgap_la-set.o .libs/libgap_la-vector.o .libs/libgap_la-compiler.o .libs/libgap_la-funcs.o .libs/libgap_la-listfunc.o .libs/libgap_la-permutat.o .libs/libgap_la-stats.o .libs/libgap_la-weakptr.o .libs/libgap_la-compstat.o .libs/libgap_la-gap.o .libs/libgap_la-listoper.o .libs/libgap_la-plist.o .libs/libgap_la-streams.o .libs/libgap_la-c_oper1.o .libs/libgap_la-lists.o .libs/libgap_la-precord.o .libs/libgap_la-string.o .libs/libgap_la-costab.o .libs/libgap_la-gasman.o .libs/libgap_la-macfloat.o .libs/libgap_la-range.o .libs/libgap_la-sysfiles.o   -L/usr/local/src/sage/sage-5.6.beta2/local/lib /usr/local/src/sage/sage-5.6.beta2/local/lib/libgmp.dylib    -install_name  /usr/local/src/sage/sage-5.6.beta2/local/lib/libgap.0.dylib -compatibility_version 1 -current_version 1.0 -Wl,-single_module

Undefined symbols:
  "_environ", referenced from:
      _libGAP_SyExecuteProcess in libgap_la-sysfiles.o

[jpflori]

comment:29

The spkg is updated (I've not commited anything as I don't want to deal with the external repository hell).

And I still do not understand what you meant after reading all the thread.
The last message basically says that using _NSGetEnviron() worked, and in other messages its told that the solution used is not to remove no-undefined but to use _NSGetEnviron().

By ld do you mean run-time linking? because I don't think its ld that takes care of that.
And passing -undefined dynamic_lookup won't let ld resolve anything at linking time.
It is just a promise that at runtime environ will be available somehow, which could perfectly not be the case if the proper libraries are not loaded.

[dimpase]

comment:30

Replying to @jpflori:

The spkg is updated (I've not commited anything as I don't want to deal with the external repository hell).

OK, it builds and seemingly works.

And I still do not understand what you meant after reading all the thread.
The last message basically says that using _NSGetEnviron() worked, and in other messages its told that the solution used is not to remove no-undefined but to use _NSGetEnviron().

No, I think it says that the library built with the -undefined dynamic_lookup option works.

By ld do you mean run-time linking? because I don't think its ld that takes care of that.

OK, here is the example:

$ cat enlib.c
extern char** environ;
char *** environ_addr (void) { return &environ; }

$ glibtool --mode=compile gcc -c enlib.c 
glibtool: compile:  gcc -c enlib.c  -fno-common -DPIC -o .libs/enlib.o
glibtool: compile:  gcc -c enlib.c -o enlib.o >/dev/null 2>&1

$ glibtool --mode=link gcc -o libenlib.la enlib.lo -rpath /tmp
glibtool: link: rm -fr  .libs/libenlib.a .libs/libenlib.la
glibtool: link: gcc -dynamiclib -Wl,-undefined -Wl,dynamic_lookup -o .libs/libenlib.0.dylib  .libs/enlib.o      -install_name  /tmp/libenlib.0.dylib -compatibility_version 1 -current_version 1.0 -Wl,-single_module
glibtool: link: dsymutil .libs/libenlib.0.dylib || :
warning: no debug symbols in executable (-arch x86_64)
glibtool: link: (cd ".libs" && rm -f "libenlib.dylib" && ln -s "libenlib.0.dylib" "libenlib.dylib")
glibtool: link: (cd ".libs" && rm -f "libenlib.0.0.0.dylib" && ln -s "libenlib.0.dylib" "libenlib.0.0.0.dylib")
glibtool: link: ar cru .libs/libenlib.a  enlib.o
glibtool: link: ranlib .libs/libenlib.a
glibtool: link: ( cd ".libs" && rm -f "libenlib.la" && ln -s "../libenlib.la" "libenlib.la" )

$ cat en.c
#include <stdio.h>
#include <crt_externs.h>
#define macenviron (*_NSGetEnviron())
extern char *** environ_addr (void); /* comes from my enlib */
int main()
{
  printf(" from the shared lib:\n  %s \n", **environ_addr());
  printf(" from macenviron :\n  %s \n", *macenviron);
}

$ glibtool --mode=link gcc -o en en.c libenlib.la 
glibtool: link: gcc -o .libs/en en.c  ./.libs/libenlib.0.0.0.dylib

$ ./en
 from the shared lib:
  NNTPSERVER=news.gmane.org 
 from macenviron :
  NNTPSERVER=news.gmane.org 

So you see, it works with -undefined dynamic_lookup just fine. (printf(%s) isn't the rght way of parsing of environ, so the output is cut at the first \n, I guess).

[jpflori]

Work Issues: integrate changes upstream

[jpflori]

Changed author from Volker Braun to Volker Braun, Jean-Pierre Flori

[jpflori]

comment:31

Volker, would you mind integrating the changes from the last version I posted "upstream" and repackage a "proper" spkg?

[vbraun]

comment:32

Done.

https://bitbucket.org/vbraun/libgap/commits/4ff0ca203c552df03a621eecaf64f1b1f2c5d691

[jpflori]

comment:33

Dima, could you give it a shot on Mac OS so that we we put it as positive review?
As far as I'm cocnerned, everything looks fine.

[jpflori]

Changed work issues from integrate changes upstream to none

[jpflori]

Changed reviewer from Jean-Pierre Flori to Jean-Pierre Flori, Dmitrii Pasechnik

[dimpase]

comment:34

good; works on OSX too, including the standalone test in sage/libs/gap/test/. Positive review.

[jdemeyer]

Merged: sage-5.8.beta0