Opened 11 years ago

Closed 11 years ago

#398 closed bug (fixed)

spielwiese compilation issues on my ARM box

Reported by: Snark Owned by: Oleksandr
Priority: minor Milestone: 3-1-4 and higher
Component: factory Version: 3-1-3
Keywords: spielwiese, build system, factory, omalloc Cc:

Description

There are two problems which prevent me to compile successfully :

(1) problem finding omalloc.h from inside factory/ : int_int.h and int_poly.h #include <omalloc/omalloc.h>, but it isn't found.

Indeed, factory/Makefile.am has : AM_CPPFLAGS = -I${builddir}/include -I${srcdir}/include ${GMP_CFLAGS} ${NTL_CFLAGS} I would suggest to add a -I${srcdir}/.. there. Why I see the problem and not you is an interesting question... different configure switches?

(2) there's a missing 'r' in libpolys/polys/monomials/p_polys.h, near line 679 : -static inline poly p_New(const ring, omBin bin) +static inline poly p_New(const ring r, omBin bin)

Attachments (5)

missing_r.patch (482 bytes) - added by julien.puydt@… 11 years ago.
Yet another missing r to add
factory_omalloc.patch (2.8 KB) - added by julien.puydt@… 11 years ago.
Make the factory/ configure&Makefiles handle a moving omalloc
singular_omalloc.patch (4.7 KB) - added by julien.puydt@… 11 years ago.
Patch to handle omalloc in spielwiese
singular_omalloc.2.patch (4.8 KB) - added by julien.puydt@… 11 years ago.
Patch to handle omalloc in spielwiese
0001-Modified-the-omalloc-configuration-for-factory-and-m.patch (5.9 KB) - added by julien.puydt@… 11 years ago.
"git format-patch -p1" version of the previous patch

Download all attachments as: .zip

Change History (34)

comment:1 Changed 11 years ago by Oleksandr

Hi! Thanks for trying spielwiese!

1) nope. one should better avoid using "${srcdir}/.." within factory as this can break the standalone factory building.

please try adding "-I${top_srcdir} -I${top_builddir}" to AM_CPPFLAGS (factory/Makefile.am)

2) okay, thanks

Regards, O.

comment:2 Changed 11 years ago by julien.puydt@…

Using -I${top_srcdir} instead of ${srcdir}/.. fails -- because the configure script which turns the Makefile.in into a Makefile isn't the top-configure, but the below-configure, for which top is inside factory/.

Perhaps a good solution would be:

  1. factory/configure has a switch like --with-omalloc=...
  2. the toplevel configure calls that configure with --with-omalloc=${top_srcdir}

That would make factory's configure work for standalone and as within singular, I think.

comment:3 Changed 11 years ago by Oleksandr

Hm, how do you build spielwiese?

Our build chain is to run the following in the *root directory*:

./autogen.sh # if needed
./configure ...
make 
make check
make install-strip

Note that this way ${top_srcdir} is the general root! I know this is a hack and your proposed solution is the correct way to deal with dependencies... Unfortunately, right now we lack manpower and try to fix bugs first.

BTW, we gladly accept patches from external contributors ;-) and will acknowledge all helping people.

Cheers

comment:4 Changed 11 years ago by julien.puydt@…

What I don't understand is why you don't have the same problem :-/

I'm using the following configure line, directly inspired from the one used by sage : ./configure --prefix=/home/jpuydt/sage-4.8.alpha6/local/ --exec-prefix=/home/jpuydt/sage-4.8.alpha6/local/ --bindir=/home/jpuydt/sage-4.8.alpha6/localbin --libdir=/home/jpuydt/sage-4.8.alpha6/locallib --libexecdir=/home/jpuydt/sage-4.8.alpha6/locallib --with-apint=gmp --with-gmp=/home/jpuydt/sage-4.8.alpha6/local/ --with-ntl=/home/jpuydt/sage-4.8.alpha6/local/ --with-NTL --without-MP --without-lex --without-bison --without-Boost --enable-gmp=/home/jpuydt/sage-4.8.alpha6/local/ --enable-Singular --enable-factory --enable-libfac --enable-IntegerProgramming? --disable-doc --with-malloc=system --includedir=/home/jpuydt/sage-4.8.alpha6/localinclude --enable-omalloc --with-external-config_h=/home/jpuydt/Singular-3-1-4/Singular/omSingularConfig.h --with-track-custom --enable-NTL --enable-Plural --with-factory --with-libfac --with-Singular=yes

My point about the root directory, which you didn't get is that factory/Makefile is generated from factory/Makefile.in by factory/configure -- and for that factory/configure, the toplevel is factory/ ; it's only for the toplevel configure that toplevel is really toplevel... this is relative!

comment:5 Changed 11 years ago by julien.puydt@…

oh, and yes, I'll try to understand what happens and provide a patch!

comment:6 in reply to:  4 Changed 11 years ago by Oleksandr

Wow, looks like you are trying to build the spielwiese Singular for Sage. Please note that the build system and some internal parts were completely changed compared to the old Singular. Moreover we do not build IntegerProgramming/?* as of now :-(

Replying to julien.puydt@…:

My point was that one shall not execute factory/configure at all since the spielwiese' ROOT/configure will call all of ROOT/omalloc/configure, ROOT/factory/configure and ROOT/libpolys/configure so that they all have the same $(top_srcdir) == ROOT! Do i miss something?

comment:7 Changed 11 years ago by julien.puydt@…

Well, the deps are not on my system but in my ~/sage-4.8.alpha6/local directory, that's why I thought giving those options would make sense.

What you're missing is that I don't run factory/configure by hand : I only run the toplevel configure. And that toplevel configure in turn runs factory/configure... and that one thinks its $(top_srcdir) is in factory/, since that's where it is -- at least that's how I see things.

I'll try to have a deeper look and perhaps provide a patch.

comment:8 Changed 11 years ago by Oleksandr

Oh, yeah sorry, i was wrong about ${top_*}. Looks like we used to use "${top_srcdir}/.." and "${top_builddir}/..", which is a hack. There were fixes by Jan Engelhardt addressing that.

It seems that we used to build factory without omalloc :-/

Any help will be much appreciated.

comment:9 Changed 11 years ago by Oleksandr

I guess you have the following line in the factory/configure output, right?

checking omalloc.h usability... no
checking omalloc.h presence... no
checking for omalloc.h... no
checking whether to use omalloc... yes
configure: WARNING: did not find omalloc.h -- install before compiling

This is the effect of configuring with "--enable-omalloc"

As a workaround: one can simply remove that option (to the main configure).

Last edited 11 years ago by Oleksandr (previous) (diff)

comment:10 Changed 11 years ago by julien.puydt@…

Oh dear. And I didn't see that! What's annoying is that the configure script doesn't make it a real error. I'll see what I can do to solve things.

comment:11 Changed 11 years ago by julien.puydt@…

Ok, I studied factory/configure.ac a little ; there's much to do, and I don't think I understand everything.

(1) Why is there a comment to say not to change the version and another to say it's really the place to change the version? Why wasn't the wrong comment patched out instead of patching a denial of the denial in!?

(2) I don't get why CFLAGS and CXXFLAGS are handled like this... see around line 90, then around line 300 : why is it so complex?

(3) Why is INT64 a configure option if it's set to the same thing anyway?

(4) What is the problem with makeheader and mkinstalldirs?

(5) What is the story behind those "path generation"?

(6) I'm surprised by the checks for stdlib.h, string.h and stdarg.h... does there still exist C compilers which don't respect the C standard?

I can probably manage to have a working patch to fix the problem with omalloc -- but since I don't understand what's done, I fear such a patch would break something else :-/

Changed 11 years ago by julien.puydt@…

Attachment: missing_r.patch added

Yet another missing r to add

Changed 11 years ago by julien.puydt@…

Attachment: factory_omalloc.patch added

Make the factory/ configure&Makefiles handle a moving omalloc

comment:12 Changed 11 years ago by julien.puydt@…

I just added two patches which make things compile correctly here ; I only have to add --with-omalloc-dir=pwd (there are backquotes here, even if trac eats them) to the configure line I quoted in comment #4.

I think the patch is pretty correct : for factory/, omalloc is an external dependency, and hence should be configure-able.

The next step if that patch suits you : I'll make the toplevel configure make sure that when --enable-omalloc is given to it, then it will give both --enable-omalloc and the correct --with-omalloc-dir=... to the sub-configure.

I hope that helps.

comment:13 Changed 11 years ago by Oleksandr

Thanks for the patches.

Unfortunately it looks like with factory_omalloc.patch it is not possible to configure by default (i.e. without omalloc) anymore :(

$ configure 
...
checking whether to use omalloc... yes
checking omalloc/omalloc.h usability... no
checking omalloc/omalloc.h presence... no
checking for omalloc/omalloc.h... no
configure: error: You asked for omalloc, but it was not found
configure: error: ..../factory/configure failed for factory

Therefore I imagine that factory configure shall NOT try to use omalloc by default.

Also, it would be nice to make top-level configure to ensure that when --enable-omalloc is given to it, then it will give both --enable-omalloc and the correct --with-omalloc-dir=... to the sub-configure as you said ;)

What do You think?

comment:14 Changed 11 years ago by julien.puydt@…

Oh, I made omalloc the default?! That was unintended ; I'll update my patch to change that.

Can you first confirm that if you --disable-omalloc, things work correctly, so we're sure that's the only problem up to now?

And yes, as I said : "The next step if that patch suits you : I'll make the toplevel configure make sure that when --enable-omalloc is given to it, then it will give both --enable-omalloc and the correct --with-omalloc-dir=... to the sub-configure.".

I'm mostly waiting for feedback about the general idea of the patch before I proceed : I prefer working with at least some hope upstream will accept it.

comment:15 in reply to:  14 Changed 11 years ago by Oleksandr

Replying to julien.puydt@…:

Can you first confirm that if you --disable-omalloc, things work correctly, so we're sure that's the only problem up to now?

yes, with "--disable-omalloc" the factory configure emits:

checking whether to use omalloc... no

and goes on.

Moreover, the patch breaks the out-source-of-tree building, :( e.g. setting "--with-omalloc-dir=$SOURCE_ROOT" misses the generated header "$BUILD_ROOT/omalloc/omConfig.h" by "$SOURCE_ROOT/omalloc/omalloc.h". This problem could be solved by specifying BOTH source and build omalloc directories... which is not nice :/

I'm mostly waiting for feedback about the general idea of the patch before I proceed : I prefer working with at least some hope upstream will accept it.

We gratefully accept any sensible contributions which do not lead to any apparent problems. Usually I test for build problems with something like: "./autogen.sh && ./configure && make distcheck".

comment:16 Changed 11 years ago by julien.puydt@…

Ok, so that makes three issues to settle:

  1. omalloc should be disabled by default ;
  2. the toplevel Makefile should cope with the new options in factory/ ;
  3. out-of-tree should work.

Of those, the third worries me, because as you point out, factory/ depends on both omalloc-as-source and omalloc-as-built. The problem stems from the fact that omalloc is both used as an external dependence and an internal feature. :-/

Perhaps I took the wrong way to fix the initial problem?

comment:17 Changed 11 years ago by julien.puydt@…

Sigh... I mostly have a working patch. Mostly. The problem is that omalloc.h uses omTables.h, which isn't available at configure-time. So basically, my patch works if I run configure, then go type make omTables.h in the omalloc/ directory, go up and re-run configure.

I'm really wondering how that could work. :-/

Changed 11 years ago by julien.puydt@…

Attachment: singular_omalloc.patch added

Patch to handle omalloc in spielwiese

comment:18 Changed 11 years ago by julien.puydt@…

I made a patch which I checked works with out-of-tree build both with and without omalloc.

comment:19 Changed 11 years ago by julien.puydt@…

Sigh... it doesn't apply anymore : I forgot to fetch&rebase :-(

I'll provide a new one when I'll have retested.

Changed 11 years ago by julien.puydt@…

Attachment: singular_omalloc.2.patch added

Patch to handle omalloc in spielwiese

comment:20 Changed 11 years ago by julien.puydt@…

Here is a rebased patch. It is known to allow out-of-tree builds both with and without omalloc.

comment:21 Changed 11 years ago by Oleksandr

Owner: changed from somebody to Oleksandr

comment:22 Changed 11 years ago by Oleksandr

Thanks.

I will test it out and report my findings later on.

comment:23 Changed 11 years ago by Oleksandr

Status: newassigned

comment:24 Changed 11 years ago by Oleksandr

Ok, it works... after fixing the test programs linking for "make check" (already done by me).

ps: could you please generate a complete git patch with an Author and commit Message?

Changed 11 years ago by julien.puydt@…

"git format-patch -p1" version of the previous patch

comment:25 Changed 11 years ago by julien.puydt@…

Here it is!

comment:26 Changed 11 years ago by julien.puydt@…

I saw my patch is in : thanks :-)

That bug can be closed, then.

comment:27 Changed 11 years ago by Oleksandr

Could You please check that after our further changes in defaults and internals it still works for You?

Thanks for contributing!

Best Regards!

comment:28 Changed 11 years ago by julien.puydt@…

I just managed to compile remote-run/malex/polished_arm_gftables, at commit 9f39e6f943814dcc86665ee80a22e3f8ff8c12fa, on my ARM box.

comment:29 Changed 11 years ago by Oleksandr

Component: dontKnowfactory
Keywords: spielwiese build system factory omalloc added
Resolution: fixed
Status: assignedclosed

Great!

Thanks for collaboration!

Note: See TracTickets for help on using tickets.