CI: Do not install readline for macOS job

[wilfwilson]

Homebrew (currently) installs the readline stuff into /usr/local/opt/readline, but GAP does not know about this directory by default, and hence it was being compiled without readline – see https://github.com/gap-system/gap/runs/2288444515 for an example:

 ┌───────┐   GAP 4.12dev built on 2021-04-07 14:44:10+0000
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-apple-darwin19.6.0-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, KernelDebug
 Loading the library and packages ...

This is presumably counter to the intentions of whoever added the macOS job, because homebrew is being explicitly told to install readline. By adding this directory to CONFIGFLAGS, GAP compiles and starts with readline. Note that CONFIGFLAGS contains --enable-debug in the workflow file by default, so I've explicitly added it back in to not change that aspect of the job.

I have created this PR more as an issue than a request, because I think there's a high probability that I am not going about this in the best way, so I am hoping that someone will suggest a cleaner way. For instance: would it be better (and if so, how?) to somehow "install" Homebrew's readline so that GAP doesn't need to be told about it? (Note: I would expect brew link readline to do the job, but it fails on my computer because apparently macOS already provides readline. If so, why doesn't GAP see it?). I know almost nothing about compilation/build systems/what readline even is.

This also raises an administrative point: some of the CI jobs are required to pass in order for a PR to be merged. This feature uses the names of certain jobs. The macOS job is one of those, however I am changing its name in this PR (the name of a job currently includes the value of any 'extra' variables). Therefore, the CI on this PR will not pass (notwithstanding #4377) until someone changes the branch protection rules for gap-system/gap to not require the currently-named macOS job. This is a slight annoyance and it would be good if we could come up with a solution that both provides useful and descriptive information in the GitHub interface, but which isn't as sensitive to changes like this PR contains. This of course might be impossible.

[wilfwilson]

Note that the CI here will probably fail until #4377 is merged, but even when it fails, the macOS job should succeed enough that we can see that readline is being properly used.

This is something of personal preference, but I'd like to see the names of CI jobs be rearranged to put the OS first (at least for the non-Ubuntu jobs, where the fact that the OS is not Ubuntu is probably the most important fact about the job – I already did this for the Cygwin job). Often I struggle (in the PR view and in the GitHub Actions tab) to work out which job is the macOS job, because these interfaces typically focus on the first part of the name. This is the macOS job in this PR:

I realise this is all 'nitpicking' etc, but seeing as we spend a lot of time looking at the result of CI runs, I think it's worth discussing how to make this as frictionless as possible 🙂. I'd be happy to make the changes myself, but I suppose I'd like to see if we can reach some sort of consensus before trying.

[fingolfin]

There are several different libraries called readline. GAP needs "GNU readline", while on macOS, there is "BSD readline" -- IIRC they have a have shared basic feature set, but we explicitly use features exclusive to GNU readline.

Since both libraries have the same name, Homebrew cannot just install them in a place where every software sees them, because it would e.g. break things that want the "native" readline.

Note that on M1 macs, Homebrew installs into /opt/homebrew, so GAP won't find anything installed via it by default (including GMP). So one needs to tell users to add suitable CPPFLAGS / LDFLAGS...

See also issue #1945

[fingolfin]

Probably it was me who added the brew install readline without thinking. I think it would be fine to just remove it -- it's actually a bonus to test building without readline in at least one config.

Improving the experience for Homebrew users on macOS would be nice, see issue #1945; it gets a bit more urgent perhaps with M1 macs

Finally, regarding renaming the CI tasks and the resulting issue with branch protection rules: This can be fixed by simply assigning a name to each task manually; the current "automatic names" were simply the quickest way to get going, but they are a bit unwieldy anyway. So, we can just define a name or NAME or JOB_NAME or whatever for each job, which either gets a full custom string, or a partially custom one. I.e., we could then replace

name: ${{ matrix.test-suites }} - ${{ matrix.extra }} - ${{ matrix.os }}

by something like

name: ${{ matrix.os }} - ${{ matrix.name }}

Patches welcome ;-).

[wilfwilson]

I've updated this PR to remove the installation of readline via homebrew. Since it wasn't being used, no behaviour has actually changed 🙂 .

I hope to make a PR making 'nicer' names for the CI jobs - we'll see how I get on with that.

[fingolfin]

I added an issue for the issues of changing CI job names, see #4388

[wilfwilson]

Thanks for doing that.