Skip to content

Actually complete renaming of CONFIGFLAGS to configflags#32

Merged
fingolfin merged 1 commit intomainfrom
mh/configflags
Apr 28, 2026
Merged

Actually complete renaming of CONFIGFLAGS to configflags#32
fingolfin merged 1 commit intomainfrom
mh/configflags

Conversation

@fingolfin
Copy link
Copy Markdown
Member

This was changed in v3 via PR #21 but then got accidentally reverted in PR #23.

I noticed this when I saw by chance that Semigroups had updated to v3 without renaming the CONFIGFLAGS input, yet this did not cause any regression; e.g. here is a CI log from that PR which clearly shows that CONFIGFLAGS has an effect.

So... it seems GitHub Actions allows specifying a list of inputs, but that list in practice is almost completely useless: it does not prevent the user from passing in unsupported inputs, and we discovered in the past that its input validation is also unreliable sigh

Anyway: if we just merge and re-tag v3, this would break the Semigroups and Digraphs CI workflow. As far as I can tell, these are the only two which currently use the CONFIGFLAGS input. So, I can think of three options:

  1. we merge this PR, and simultaneously update the CI of semigroups and Digraphs (on all their branches which use gap-actions/build-pkg@v3 to use configflags)
  2. we un-document that v3 renamed CONFIGFLAGS to configflags and redo it in v4 (or not at all, it's somewhat niche anyway)
  3. we allow both configflags and CONFIGFLAGS (but without documenting it)

Would be good to hear what @james-d-mitchell thinks. None of this is his fault, so I'd like to minimize impact on him and his wonderful teams.

@fingolfin fingolfin requested a review from stertooy April 10, 2026 07:38
Comment thread action.yml
@stertooy
Copy link
Copy Markdown
Contributor

Ugh, I must have messed up a rebase or something.

I'm fine with the 1st option, but that seems a lot of work for something that's not necessary.

I'm not a huge fan of the 2nd option, because I'd like to keep naming consistent. Not just within this action, but also with other actions, since e.g. setup-gap's input is also called configflags since v3

Perhaps a variation of your 3rd suggestion:

  • We keep the README as is, but pass on both configflags and CONFIGFLAGS
  • We add some extra validation like here Throw error on usage of obsolete inputs setup-gap#65, but for the CONFIGFLAGS input we keep the default as '' and throw a deprecation warning if it's non-empty. (although that deprecation warning might get on people's nerves too...)

@james-d-mitchell
Copy link
Copy Markdown

I'm happy with whatever the best solution is for you guys @fingolfin and @stertooy, I don't think it'd be much work to update the ci of Semigroups and digraphs to use configflags instead of CONFIGFLAGS I think I might have started making this change already last week

@fingolfin
Copy link
Copy Markdown
Member Author

fingolfin commented Apr 28, 2026

Sorry, I dropped the ball on this. How about we go with 1, in the hopes that this will minimize work for everyone, and based on the assumption that only digraphs and semigroups are affected. That is:

  1. merge this PR here to change it to configflags
  2. prepare PRs to semigroups and digraphs to make the change there
  3. tag a new version here, and open those PRs at the same time
  4. hope that @james-d-mitchell and his team can quickly merge those (and possibly backport / cherry-pick to other branches).

@fingolfin fingolfin marked this pull request as ready for review April 28, 2026 09:09
@fingolfin fingolfin merged commit ea5a215 into main Apr 28, 2026
28 checks passed
@fingolfin fingolfin deleted the mh/configflags branch April 28, 2026 11:28
james-d-mitchell pushed a commit to digraphs/Digraphs that referenced this pull request Apr 28, 2026
james-d-mitchell pushed a commit to semigroups/Semigroups that referenced this pull request Apr 29, 2026
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

Successfully merging this pull request may close these issues.

3 participants