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

using installed headers, not ones in src/ ? - no version.h #21

Closed
dimpase opened this issue Oct 17, 2022 · 26 comments
Closed

using installed headers, not ones in src/ ? - no version.h #21

dimpase opened this issue Oct 17, 2022 · 26 comments

Comments

@dimpase
Copy link
Member

dimpase commented Oct 17, 2022

I see that it uses headers in src/, but these come without version.h, only version.h.in.

@dimpase
Copy link
Member Author

dimpase commented Oct 17, 2022

This happens with crypting shipped with GAP 4.12.0.
There you don't even have an option to use CFLAGS to pass extra -I....

@dimpase
Copy link
Member Author

dimpase commented Oct 17, 2022

A workaround is to add the location of installed GAP includes (with /gap suffix) to GAC_CFLAGS in sysinfo.gap.

@dimpase
Copy link
Member Author

dimpase commented Oct 17, 2022

@fingolfin - do you know a quick way to fix it?

@fingolfin
Copy link
Member

fingolfin commented Oct 17, 2022

I don't understand: what is the issue? Can you please paste the full error message you are getting?

@dimpase
Copy link
Member Author

dimpase commented Oct 18, 2022

It's due to Sage's way to install gap packages - we are separating building GAP with core packages from building more GAP packages. With GAP 4.11, gac used headers in src/, and was happy with it. This is now broken,
as version.h is no longer there. In fact, it's in build/, as well as (after running make install-headers)
in the $prefix/install/gap/.

Can gac simply use $prefix/install/gap/ ? Or it still needs src/, too?

@fingolfin
Copy link
Member

I still don't understand what's going on. Please provide some details:

  • which version of GAP are you using? GAP 4.12.0? Or a dev version?
  • which version of crypting are you using?
  • you have used our unmodified make install to install GAP to $prefix (resp. used some distro or Sage package which does that)?
  • if so, please report the output of ls -R ${prefix}/include/gap ${prefix}/lib/gap/
  • if so, please report the full output of the following command, executed in the crypting directory: ./configure --with-gaproot=${prefix}/lib/gap && make clean && make V=1
  • if you did not use make install, please clarify what you setup is

@fingolfin
Copy link
Member

fingolfin commented Oct 18, 2022

Also:

  • which gac are you using? ${prefix}/bin/gac ?
  • from some what you say, it also almost sounds as if you have two versions of GAP, one installed via make install and one in "classic" mode where you use it from inside its source directory, and you are mixing the two? That then can't work

@dimpase
Copy link
Member Author

dimpase commented Oct 18, 2022

Please provide some details:

* which version of GAP are you using? GAP 4.12.0? Or a dev version?

4.12.0

(but out setup is still mostly what's left over from GAP 4.11 - the question is by how much make install has improved, so that
we can stop using our hacks)

* which version of `crypting` are you using?

the one bundled with 4.12.0 (I gather it's not very new).

you have used our unmodified make install to install GAP to $prefix (resp. used some distro or Sage package which does that)?

no, we don't run make install, as it's too picky (something like it only works after building of docs is done). Instead, after running make all, we do the following

# GAP's "make install" is work in progress; we use bits and pieces of it
# but we install many things manually.
make install-headers install-libgap

# Install config.h, which is not currently handled by `make install-headers`
install build/config.h "$SAGE_LOCAL/include/gap"

# Now install the gap executable as "gap-bin"; it will be called normally
# through our wrapper script that sets the appropriate GAP_ROOT
SAGE_BIN="$SAGE_LOCAL/bin"
mkdir -p "$SAGE_DESTDIR$SAGE_BIN" 

./libtool --mode=install install gap "$SAGE_DESTDIR$SAGE_BIN/gap-bin"

./libtool --mode=install install gac "$SAGE_DESTDIR$SAGE_BIN/gac"

# Now copy additional files GAP needs to run (and a few optional bits) into
# GAP_ROOT; we don't need everything from the source tree
sdh_install bin doc grp lib src tst sysinfo.gap "$GAP_ROOT"

# GAP's copy of libtool is also used by the toolchain for build GAP packages
# (i.e. by gac)
sdh_install libtool "$GAP_ROOT"

# Install only the minimal packages GAP needs to run
sdh_install pkg/gapdoc pkg/primgrp pkg/smallgrp pkg/transgrp "$GAP_ROOT"/pkg

# Install additional packages that are not strictly required, but that are
# typically "expected" to be loaded: These are the default packages that are
...

what we notice is that gac and sysinfo.gap installed have no idea about the things installed by make install-*.

Sorry for this all being fuzzy. I'll ask a meaningful question in the next comment

@dimpase
Copy link
Member Author

dimpase commented Oct 18, 2022

Presently

make
make install

errors out with

[gap-4.12.0] install: ./doc/dev/*.css: No such file or directory
[gap-4.12.0] install: ./doc/dev/*.html: No such file or directory
[gap-4.12.0] install: ./doc/dev/*.js: No such file or directory
[gap-4.12.0] install: ./doc/dev/*.txt: No such file or directory
[gap-4.12.0] install: ./doc/dev/*.pdf: No such file or directory
[gap-4.12.0] install: ./doc/dev/*.six: No such file or directory
[gap-4.12.0] install: ./doc/dev/*.lab: No such file or directory
[gap-4.12.0] make[3]: *** [install-doc] Error 71
[gap-4.12.0] make[3]: *** Waiting for unfinished jobs....

I know that make doc before make install allows make install to complete. However, make doc is very slow, is there a
quicker way to enable make install ?

@fingolfin
Copy link
Member

Yeah, the doc/dev bug is known (and quite stupid sigh), and already fixed in both master and stable-4.12.0, see gap-system/gap#5091 -- as such the fix will be in GAP 4.12.1, to be released this week.

Once you have that, you should be able to use make install just fine, though there are a few other bugs in there -- I would recommend to wait a few days and try 4.12.1. Or maybe apply the patch now and try make install (should allow you to remove most of your custom script and things should just work), and then you'll be ready to switch to 4.12.1 when it is there.

@dimpase
Copy link
Member Author

dimpase commented Oct 18, 2022 via email

@dimpase
Copy link
Member Author

dimpase commented Oct 19, 2022

As we don't have make install for crypting, we need to emulate it...

What is the supposed directory structure when we install crypring using GAP installed with ${prefix}, with
./configure --with-gaproot=${prefix}/lib/gap etc. ? Is it

${prefix}/share/gap/pkg/crypting/...    # for sources etc
${prefix}/lib/gap/pkg/crypting/bin/...    # for binary stuff

Or is it different, or more complicated ?
(we also created symlinks with Sagemath, linking that bin/ somewhere)

@fingolfin
Copy link
Member

The package needs to be in a single directory currently. Your choice whether to put it into ${prefix}/lib/gap/pkg/crypting or ${prefix}/share/gap/pkg/crypting but since it'll contain architecture specific files, lib makes more sense.

@dimpase
Copy link
Member Author

dimpase commented Nov 3, 2022

There is actually another problem we have with crypting, while trying to use the result of make install. It's the header location, which is <...>/include/gap, and not <...>/include, where <...> is the directory used as the argument in GAP's installation ./configure --prefix=<...> call.

While it's possible to add -I<...>/include/gap to C(PP)FLAGS, this is just not right. E.g. if GAP is installed in a standard location,
e.g /usr/ or /usr/local/, then it's rather unfortunate to still have to add -I....

One way (and the correct one, IMHO) would be to add gap/ as prefix to all includes, i.e. #include "gap/version" rather than #include "version.h", etc.
This certainly means that all GAP packages needing GAP headers will need to do this, which is some work...

@dimpase
Copy link
Member Author

dimpase commented Nov 3, 2022

My previous comment is somewhat misplaced, as in this particular case it's about calling gac. A way to remedy the problem I described, in an ugly way, would be to make sure the correct -I<...>/include/gap is added to sysinfo.gap. For some reason the latter isn't at all aware about the GAP's install prefix.

@fingolfin
Copy link
Member

There is actually another problem we have with crypting, while trying to use the result of make install.

Please describe the actual problem (not just what you think is the cause of the problem). Specifically: what is your input, what is the error you get? Please, the full output.

As it is, I just tried make install with GAP 4.12.1, then did the following to build crypting, and it worked fine, no need for any extra FLAGS or else:

cp -r pkg $libdir/lib/gap
cd $libdir/lib/gap/pkg/crypting
./configure $libdir/lib/gap   # actually could have omitted the argument in this case
make

It's the header location, which is <...>/include/gap, and not <...>/include, where <...> is the directory used as the argument in GAP's installation ./configure --prefix=<...> call.

While it's possible to add -I<...>/include/gap to C(PP)FLAGS, this is just not right.

You should't have to add anything. The package build system automatically does the right thing, based on the content of sysinfo.gap.

@fingolfin
Copy link
Member

My previous comment is somewhat misplaced, as in this particular case it's about calling gac. A way to remedy the problem I described, in an ugly way, would be to make sure the correct -I<...>/include/gap is added to sysinfo.gap. For some reason the latter isn't at all aware about the GAP's install prefix.

It absolutely does encode it, unless you patched it out or are not using the sysinfo.gap provided by make install...

@fingolfin
Copy link
Member

Just to be clear: my long term plan is to allow packages to use either #include <gap_all.h> (already possible since GPA 4.11.1) or #include <gap/HEADER.h> (hopefully possible with GAP 4.13, see also gap-system/gap#5187), at their choice and convenience. (Actually, for almost all packages the right thing to do is to use #include <gap_all.h> because we do not guarantee anything about most of the headers, and functions can move around etc. -- so including specific header files should only be done if there are not alternatives and one knows what one is doing).

But this should not be done as an overnight breaking change. So for some time all the different ways in use by packages to access GAP headers need to keep working, until at least all distributed packages have been adapted (there are of course further packages which are not distributed, which then might break... I try my best to reach out and prevent it, but in the end, there is a limit to what I can do).

Since this needs to take all packages into account, it's a slow process. We are pretty close now, due to me working on it on the side, mostly without any aid, over the past couple years, updating tons of packages to have sane build systems, avoid various problematic practices, use sane includes etc.; and then coaxing package maintainers to merge my PRs, make releases (and in many cases taking over maintainership and making those releases myself). But it is a slow process. Also, package authors must either be willing to drop support for older GAP versions, or come up with a way to stay compatible with both old and new versions. More realistically, someone needs to provide them with such a compatibility solution -- so far this again is something which mostly I am the only one working towards.

But in all of this, I really don't see how crypting is a specific problem. It seems to work fine both with a "classic" GAP installation and the new make install. The only time there are problems is when downstream packagers decide to modify our build system and scripts without regard for compatibility (I have some sympathy for that, as for the longest time there was no good alternative), and without consulting with us (I have less sympathy for that, esp. when certain behavior changing modifications are made and the result is distributed to users without a notice about the modifications, and we then get to deal with the bug reports...)

@dimpase
Copy link
Member Author

dimpase commented Nov 4, 2022

OK, I probably don't understand how (in)complete make install is. Am I correct that it installs enough data for all packages to be installable using the installed data only (except that data is scattered across a number of directories)?

@dimpase
Copy link
Member Author

dimpase commented Nov 4, 2022

With GAP installed via make install, what should be the value of GAP_ROOT for the packages needing compilation (with or without gac) ?

@fingolfin
Copy link
Member

With GAP installed via make install, what should be the value of GAP_ROOT for the packages needing compilation (with or without gac) ?

The directory containing sysinfo.gap, which for an "installed" GAP is $libdir/lib/gap

@dimpase
Copy link
Member Author

dimpase commented Nov 4, 2022

Thanks. It appears that everything is installing and testing mostly OK, if I just drop the old Sage's workarounds of installing stuff in
${prefix}/share/gap.

@dimpase
Copy link
Member Author

dimpase commented Nov 4, 2022

Almost done, but here is a package importing problem I have that breaks make testinstall.
The 1st run of LoadAllPackages(); ends with

Loading  MapClass 1.4.6 (A Package For Mapping Class Orbit Computation)
by Adam James,
   Kay Magaard,
   Sergey Shpectorov (https://web.mat.bham.ac.uk/S.Shpectorov/index.html), and
   Helmut Volklein.
maintained by:
   Sergey Shpectorov (https://web.mat.bham.ac.uk/S.Shpectorov/index.html).
Homepage: https://gap-packages.github.io/MapClass
Report issues at https://github.com/gap-packages/MapClass/issues
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `Filename' on 2 arguments
The 1st argument is 'fail' which might point to an earlier problem
 at /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/local/share/gap/lib/methsel2.g:249 called from
<function "HANDLE_METHOD_NOT_FOUND">( <arguments> )
 called from read-eval loop at /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/local/lib/gap/pkg/packagemanager/gap/PackageManager.gd:295
type 'quit;' to quit to outer loop

and the 2nd run (after leaving the brk> loop) succeeds.

In a similar vein, the 1st run of LoadAllPackages(:reversed); ends with

Loading  polymaking 0.8.6 (A package for using polymake in GAP)
by Marc Roeder (roeder.marc@gmail.com).
maintained by:
   Marc Roeder (roeder.marc@gmail.com) and
   The GAP Team (support@gap-system.org).
Homepage: https://gap-packages.github.io/polymaking/
Report issues at https://github.com/gap-packages/polymaking/issues
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `Filename' on 2 arguments
The 1st argument is 'fail' which might point to an earlier problem
 at /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/local/share/gap/lib/methsel2.g:249 called from
<function "HANDLE_METHOD_NOT_FOUND">( <arguments> )
 called from read-eval loop at /home/scratch/scratch2/dimpase/sage/sagetrac-mirror/local/lib/gap/pkg/packagemanager/gap/PackageManager.gd:295
type 'quit;' to quit to outer loop

and the 2nd run ((after leaving the brk> loop) succeeds.

Some name clash somewhere - how to debug this?

@dimpase
Copy link
Member Author

dimpase commented Nov 4, 2022

Since this needs to take all packages into account, it's a slow process.

It appears that many (all?) packages don't have their own make install (needed for packages with compiled components).
Is there anything planned in this direction?

@dimpase
Copy link
Member Author

dimpase commented Nov 4, 2022

See also gap-packages/PackageManager#105 - which is the same as
gap-packages/PackageManager#106

PackageManager needs to be prepared for being unleashed in the aftermath of GAP's make install...

@fingolfin
Copy link
Member

Thanks for opening the PackageManager issue, I just commented on it.

Regarding make install for packages: There are multiple ideas I have how to tackle this, but I currently don't have the time (and energy). If someone else wants to open an issue at the GAP repository to get the ball rolling, fine by me. It's something I hope we can tackle in GAP 4.13 or 4.14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants