Add --add-package-config option to BuildPackages.sh

[isuruf]

Please use the following template to submit a pull request, filling
in at least the "Text for release notes" and/or "Further details".
Thank You!

Description

Text for release notes

If this pull request should be mentioned in the release notes,
please provide a short description matching the style of the GAP
Changes manual.

Add --add-package-config-<PACKAGENAME>="<CONFIG_ARGS>" option to BuildPackages.sh where <CONFIG_ARGS> are passed through to the configure script of package <PACKAGENAME>.

Further details

If necessary, please provide further details here.

Checklist for pull request reviewers

• proper formatting

If your code contains kernel C code, run clang-format on it; the
simplest way is to use git clang-format, e.g. like this (don't
forget to commit the resulting changes):

git clang-format $(git merge-base master)

• usage of relevant labels

  1. either release notes: not needed or release notes: to be added
  2. at least one of the labels bug or enhancement or new feature
  3. for changes meant to be backported to stable-4.X add the backport-to-4.X label
  4. consider adding any of the labels build system, documentation, kernel, library, tests

• runnable tests

• adequate pull request title

• well formulated text for release notes

• relevant documentation updates

• sensible comments in the code

[isuruf]

cc @james-d-mitchell

[coveralls]

Coverage increased (+0.0006%) to 84.419% when pulling fd77f29 on isuruf:BuildPackages into f5edc9e on gap-system:master.

[ChrisJefferson]

I'm not a huge fan of this, because I worry we will gain tens of special options for different packages, and I was hoping to push the BuildPackages.sh towards not refering to any package explictly.

I wonder if it would be reasonable to have something like:

--add-package-config PACKAGENAME="flags"

for example:

--add-package-config Semigroups="--with-external-planarity"

Where the name of the package ('Semigroups' in this case) would be extracted from PackageInfo.g

[codecov]

Codecov Report

Merging #3542 into master will decrease coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3542      +/-   ##
==========================================
- Coverage   84.66%   84.59%   -0.07%     
==========================================
  Files         698      698              
  Lines      345819   345819              
==========================================
- Hits       292791   292549     -242     
- Misses      53028    53270     +242

Impacted Files	Coverage Δ
lib/process.gi	26% <0%> (-9%)	⬇️
lib/float.gd	93.54% <0%> (-5.38%)	⬇️
lib/info.gi	79.03% <0%> (-2.63%)	⬇️
lib/grpfp.gi	78.32% <0%> (-2.1%)	⬇️
lib/relation.gi	70.24% <0%> (-1.91%)	⬇️
lib/oper.g	84.66% <0%> (-1.21%)	⬇️
lib/dict.gi	77.68% <0%> (-1.06%)	⬇️
lib/magma.gi	86.93% <0%> (-0.96%)	⬇️
lib/package.gi	74.52% <0%> (-0.93%)	⬇️
lib/invsgp.gi	75.61% <0%> (-0.93%)	⬇️
... and 15 more

[ChrisJefferson]

I mean where in BuildPackages.sh you read this file, you could instead just put the contents of the file, so we don't ha e to think about where to put it.

[isuruf]

I mean where in BuildPackages.sh you read this file, you could instead just put the contents of the file, so we don't ha e to think about where to put it.

I don't know how to invoke gap that way. Do you have an example?

[fingolfin]

@isuruf Chris is talking about a here document (wikipedia). For an example in the context of GAP, take a look here: https://github.com/gap-system/ReleaseTools/blob/45f3dbfc9ec1f2428cee455bddc79bd7f76ec9de/release#L254

[isuruf]

@fingolfin, @ChrisJefferson, can you review again?

[fingolfin]

I think we can merge this with one minor change. I can make that change later, too.

Sorry for being so slow :/

[fingolfin]

I am fine with this. Thank you very much for your patience, and sorry once again for taking so long. We'll include this to 4.11, though!

One last request: could you please squash this into a single commit, with suitable commit message (and perhaps remove that one stray empty line 200 which is being inserted now)?

[isuruf]

Done.

[fingolfin]

I have restarted the failed AppVeyor build, once that finishes, we can merge this.