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

Compiling the package manual may require the package to be loaded (and hence the package compiled) #74

Closed
wilfwilson opened this issue Feb 8, 2021 · 4 comments

Comments

@wilfwilson
Copy link
Member

This issue is possibly not the problem of ReleaseTools, but I'll raise it anyway.

For some packages (like Semigroups, guava...), the makedoc.g file contains the line GAPDocManualLab(<pkg_name>), which loads the package. (GAPDocManualLab is a GAP library function).

For some packages (like Semigroups), the package must be compiled for it to load. Therefore, running makedoc.g in the release script will either fail, or the package must have already been compiled - but this means that we'll be including unwanted files in the released package archive.

I see several ways to deal with this:

  • Perhaps packages don't need to be calling GAPDocManualLab in makedoc.g.
  • Perhaps GAPDocManualLab could be modified to not need to load the package.
  • (This is @james-d-mitchell's current solution): build the documentation ourselves in the .release script, and then run make distclean, and delete the makedoc.g file so that the release script doesn't attempt to build the documentation again (which would fail).
  • release-gap-package could run autogen.sh after building the documentation, and then between building the documentation and running autogen.sh, the script could attempt to run make distclean. Therefore we would just need to make sure the package is properly built in .release, and then release-gap-package would uncompile it for us after the documentation is built via make distclean.
    • Perhaps release-gap-package could even offer the option of building the package, for this purpose.

I would be interested to hear anyone's opinion/advice on this matter.

@fingolfin
Copy link
Member

Some remarks:

  1. if semigroups was using AutoDoc to compile its manual (just for that -- no need to use any of its other features), the problem would not exist
  2. barring that, its makedoc.g could call GAPDocManualLabFromSixFile instead of GAPDocManualLab to avoid the issue (that's what AutoDoc does)
  3. Of course the "hack" where you remove makedoc.g in .release works, but the drawback (if I understand it correctly) is that then the release tarball is missing this file, which is IMHO not great (I am sure Debian folks would be unhappy, too)
  4. A better variant of 3. would be to add an option --ignore-makedoc or --no-makedoc or so to release-gap-package which does what the name says, and then document it explicitly as saying that then .release should build the docs -- however, IMHO this is really inferior to options 1.+2.: the idea after all is that makedoc.g should "just work"
  5. Running make distclean sounds like a good idea. Caveat: not all packages support it. (I guess one could use the exit code of make -q distclean to test for it ... )
  6. I don't like the idea of trying to build the package at all; this can be arbitrarily complicated, and require all kinds of special flags etc. -- it seems to be a very fringe need, and even in the case of Semigroups, it seems to be entirely avoidable (see options 1./2.)

Personally I think it'd be best if 1./2. were done in Semigroups and other "affected" packages, and 5 in release-gap-package

@james-d-mitchell
Copy link

Thanks @fingolfin I didn't know that GAPDocManualLabFromSixFile existed, so I'm happy to do this instead of GAPDocManualLab. I agree that 3, is not good, and will cease doing it if we can make 2 work!

@fingolfin
Copy link
Member

Something like GAPDocManualLabFromSixFile("Semigroups", "doc/manual.six"); ought to do the trick. (Untested!)

@wilfwilson
Copy link
Member Author

Thanks for your remarks @fingolfin. I've made issues to Semigroups (semigroups/Semigroups#745) and to this repo (#76) that correspond to the solution you identified.

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

3 participants