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

Use MarkAllButFirstSubBags for T_BIPART #1003

Merged
merged 3 commits into from
Mar 9, 2024

Conversation

fingolfin
Copy link
Contributor

T_BIPART consists of a general pointer not managed by GAP's GC, plus two bag references (which may be NULL). Hence we can use MarkAllButFirstSubBags instead of a custom marking function.

This prepares Semigroups for gap-system/gap#5662, but also works in previous GAP versions.

T_BIPART consists of a general pointer not managed by GAP's GC,
plus two bag references (which may be NULL). Hence we can use
MarkAllButFirstSubBags instead of a custom marking function.
@fingolfin
Copy link
Contributor Author

(I based this on stable-5.3 but accidentally targeted main now... but the target of the PR can of course be changed as you need it, @james-d-mitchell )

@james-d-mitchell
Copy link
Collaborator

Thanks @fingolfin either branch is fine. Any idea why the ci is failing looks related to recent changes in GAP and not related to this pr at all

@fingolfin
Copy link
Contributor Author

The CI failure are due to the stable-4.12 tests using the latest package distro, which has a newer GAPDoc, which makes one of the tests in stable-4.12 fail.

The ideal solution would be to test against GAP 4.12.2 (the release version) with its bundled packages, instead of against stable-4.12 plus "latest" packages. See gap-actions/setup-gap#24.

This is something someone could look into during GAP Days next week (but realistically I think of the remaining "active" people only @james-d-mitchell, @ChrisJefferson and myself qualify... I can see if I can find a "young volunteer" to teach them, but I don't see it yet. Ah well)

A quicker workaround for the interim would be to insert a step between building GAP and running its test suite where you just remove (or update) the offending .tst file.

@fingolfin
Copy link
Contributor Author

@james-d-mitchell do you think you might have time to get this into a new Semigroups release within the next couple days so we could release GAP 4.13.0 next week? Not trying to pressure you here at all, just trying to gauge what is still possible and what not :-)

@james-d-mitchell
Copy link
Collaborator

@fingolfin yes definitely, I can make a release, would like to see the CI passing first if possible. Happy to have the easy resolution of just deleting the offending files if we're using the wrong version version of GAP.

@james-d-mitchell james-d-mitchell added the gap-compatibility A label for PRs or issues that are related to compatibility with changes in GAP label Mar 7, 2024
@fingolfin
Copy link
Contributor Author

Basically if running the tests in stable-4.12, delete GAP's tst/testinstall/package.tst and that should settle it (or just always delete that file)

@james-d-mitchell james-d-mitchell merged commit dc6462b into semigroups:main Mar 9, 2024
13 of 14 checks passed
@fingolfin fingolfin deleted the mh/mark branch March 12, 2024 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gap-compatibility A label for PRs or issues that are related to compatibility with changes in GAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants