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.

Sign up for GitHub

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

Add CI job against GAP master #995

Merged
merged 18 commits into from
May 31, 2024
Merged

Add CI job against GAP master #995

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
9b7bec4
Add CI job against GAP master
lgoettgens May 22, 2024
8e25c7a
fix
lgoettgens May 22, 2024
be70ccb
more
lgoettgens May 22, 2024
8aef207
moore
lgoettgens May 22, 2024
7c95b1d
try GAP 4.13
lgoettgens May 22, 2024
8d484ab
Address comments
lgoettgens May 25, 2024
2b0fdfc
Full matrix
lgoettgens May 25, 2024
84a46d6
Add macos deps
lgoettgens May 25, 2024
2a85cbe
changes
lgoettgens May 25, 2024
d3b566a
try to work around some nightly issue
lgoettgens May 25, 2024
c5d751c
Remove `touch`
lgoettgens May 27, 2024
fad961c
Add fix for macOS
lgoettgens May 27, 2024
db25a37
Update src/setup.jl
lgoettgens May 27, 2024
c6b3314
Update src/setup.jl
lgoettgens May 27, 2024
4c85966
Update .github/workflows/gap.yml
lgoettgens May 27, 2024
5a3b2bc
Reduce number of jobs
lgoettgens May 28, 2024
d61e12a
fix
lgoettgens May 28, 2024
f9756a4
Update etc/run_with_override.jl
fingolfin May 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions .github/workflows/gap.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
name: CI with GAP

on:
push:
branches:
- 'master'
- 'release-*'
tags: '*'
pull_request:

concurrency:
# group by workflow and ref; the last slightly strange component ensures that for pull
# requests, we limit to 1 concurrent job, but for the default repository branch we don't
group: ${{ github.workflow }}-${{ github.ref }}-${{ github.ref_name != github.event.repository.default_branch || github.run_number }}
# Cancel intermediate builds, but only if it is a pull request build.
cancel-in-progress: ${{ startsWith(github.ref, 'refs/pull/') }}

jobs:
test:
name: Julia ${{ matrix.julia-version }} - GAP ${{ matrix.gap-version }} - ${{ matrix.os }}
runs-on: ${{ matrix.os }}
timeout-minutes: 20
continue-on-error: ${{ matrix.julia-version == 'nightly' }}
strategy:
fail-fast: false
matrix:
gap-version:
- 'master'
- 'stable-4.13'
- 'v4.13.0' # latest release
fingolfin marked this conversation as resolved.
Show resolved Hide resolved
julia-version:
- '1.6'
- '1.10'
lgoettgens marked this conversation as resolved.
Show resolved Hide resolved
- 'nightly'
julia-arch:
- x64
os:
- ubuntu-latest
- macOS-latest
exclude:
# Remove a few macOS jobs (the number we can run in parallel is limited)
lgoettgens marked this conversation as resolved.
Show resolved Hide resolved
- julia-version: '1.6'
os: macOS-latest

steps:
- uses: actions/checkout@v4
- name: "Set up Julia"
uses: julia-actions/setup-julia@v2
with:
version: ${{ matrix.julia-version }}
arch: ${{ matrix.julia-arch }}
- name: Checkout GAP
uses: actions/checkout@v4
with:
repository: 'gap-system/gap'
ref: ${{ matrix.gap-version }}
path: 'GAPROOT'
- name: "Install dependencies (for macOS)"
if: runner.os == 'macOS'
run: |
brew install autoconf zlib # gmp pkg-config
- name: "Build GAP"
run: |
mv GAPROOT /tmp/GAPROOT
cd /tmp/GAPROOT
./autogen.sh
./configure
make -j`nproc`
- name: "Override bundled GAP"
run: |
julia --proj=override etc/setup_override_dir.jl /tmp/GAPROOT /tmp/gap_jll_override
- name: "Run tests"
run: |
julia --proj=override etc/run_with_override.jl /tmp/gap_jll_override --depwarn=error -e "using Pkg; Pkg.test(\"GAP\")"

19 changes: 12 additions & 7 deletions etc/run_with_override.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,23 @@ gapoverride = abspath(gapoverride)
#
@info "Install needed packages"
using Pkg
Pkg.add(["GAP_jll"])
Pkg.add(["GAP_lib_jll"])
Pkg.develop(path=dirname(dirname(@__FILE__)))
Pkg.add(["GAP_jll", "GAP_lib_jll"])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changed order of additions makes a little bit faster to precompile

Pkg.instantiate()

import GAP_lib_jll

#
#
#
function add_jll_override(depot, pkgname, newdir)
function add_jll_override(depot, pkgname, pkguuid, newdir)
pkgid = Base.identify_package("$(pkgname)_jll")
uuid = string(pkgid.uuid)
# does not work with julia 1.12-dev
fingolfin marked this conversation as resolved.
Show resolved Hide resolved
# pkguuid = string(pkgid.uuid)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This threw some (seemingly random) segfaults with nightly. But only if running from the console, not if copying everything to the reply.
Thus I took the easy solution and added the UUID to the arguments instead

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried with Julia master and when running this code I actually got

ERROR: LoadError: type Nothing has no field uuid

which would suggest pkgid === nothing, but then also the touch below would be borked?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you print the pkgid, this succeeds (and prints something non-trivial), but i then get a segfault instead of a LoadError when accessing pkgid.uuid

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed: printing the pkgid makes it proceed -- that seems like a big regression in Julia, printing a value shouldn't change it form nothing to something, should it? In particular since this works in the REPL (without printing):

pkgid = Base.identify_package("GAP_jll"); pkgid.uuid

Perhaps we should open an issue?

Anyway, so I inserted the print and it proceeds, but I don't get a crash, I get this:

[ Info: Created temporary depot at /var/folders/d_/1zss2fnd6xdgclqnj8jj_27m0000gp/T/jl_7ABU3q
pkgname GAP has pkgid Base.PkgId(Base.UUID("5cd7a574-2c56-5be2-91dc-c8bc375b9ddf"), "GAP_jll")
pkgname GAP_lib has pkgid Base.PkgId(Base.UUID("de1ad85e-c930-5cd4-919d-ccd3fcafd1a3"), "GAP_lib_jll")
Precompiling GAP...
Info Given GAP was explicitly requested, output will be shown live
ERROR: LoadError: ArgumentError: Package GAP_jll [5cd7a574-2c56-5be2-91dc-c8bc375b9ddf] is required but does not seem to be installed:
 - Run `Pkg.instantiate()` to install all recorded dependencies.

which makes no sense, as we run Pkg.instantiate(). But I've inserted another Pkg.instantiate() invocation right after the add_jll_override calls.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reporting it upstream, it seems we've hit an IMHO rather serious looking code generation bug in Julia.

For now we can keep this workaround, but let's remove it again when they fix their bug.

mkpath(joinpath(depot, "artifacts"))
open(joinpath(depot, "artifacts", "Overrides.toml"), "a") do f
write(f, """
[$(uuid)]
[$(pkguuid)]
$(pkgname) = "$(newdir)"
""")
end
Expand All @@ -40,8 +42,11 @@ tmpdepot = mktempdir(; cleanup=true)
@info "Created temporary depot at $(tmpdepot)"

# create override file for GAP_jll
add_jll_override(tmpdepot, "GAP", gapoverride)
add_jll_override(tmpdepot, "GAP_lib", gapoverride)
add_jll_override(tmpdepot, "GAP", "5cd7a574-2c56-5be2-91dc-c8bc375b9ddf", gapoverride)
add_jll_override(tmpdepot, "GAP_lib", "de1ad85e-c930-5cd4-919d-ccd3fcafd1a3", gapoverride)

# HACK: use the documentation from GAP_lib_jll instead of rebuilding it
run(`ln -sf $(abspath(GAP_lib_jll.find_artifact_dir(), "share", "gap", "doc")) $(abspath(gapoverride, "share", "gap", "doc"))`)
fingolfin marked this conversation as resolved.
Show resolved Hide resolved

# prepend our temporary depot to the depot list...
withenv("JULIA_DEPOT_PATH"=>tmpdepot*":", "FORCE_JULIAINTERFACE_COMPILATION" => "true") do
Expand Down
2 changes: 1 addition & 1 deletion etc/setup_override_dir.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function gmp_artifact_dir()
hash = Base.SHA1(meta["git-tree-sha1"])
if !artifact_exists(hash)
dl_info = first(meta["download"])
download_artifact(hash, dl_info["url"], dl_info["sha256"])
Pkg.Artifacts.download_artifact(hash, dl_info["url"], dl_info["sha256"])
fingolfin marked this conversation as resolved.
Show resolved Hide resolved
end
return artifact_path(hash)
end
Expand Down
1 change: 1 addition & 0 deletions src/setup.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ function regenerate_gaproot()
# add the necessary flags to link against libgap
gap_lib = joinpath(gap_prefix, "lib")
sysinfo["GAP_LDFLAGS"] = "-L$(gap_lib) -lgap"
Sys.isapple() && (sysinfo["GAC_LDFLAGS"] = " -bundle") # Remove this line once the GAP_jll build recipe is fixed

GAP_VERSION = VersionNumber(sysinfo["GAP_VERSION"])
gaproot_packages = joinpath(Base.DEPOT_PATH[1], "gaproot", "v$(GAP_VERSION.major).$(GAP_VERSION.minor)")
Expand Down
Loading