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

Migrate CI to GitHub Actions #637

Merged
merged 16 commits into from
Oct 24, 2021
Merged

Conversation

dlozeve
Copy link
Contributor

@dlozeve dlozeve commented Oct 23, 2021

Fixes #636.
Fixes #80.

There are two workflows, one using Gambit 4.9.3 and one using Gambit master. At present, Gambit master fails to compile (although it uses the exact same build commands as 4.9.3), I'll have to investigate more.

@dlozeve dlozeve force-pushed the github-actions branch 3 times, most recently from f3e6a42 to a776062 Compare October 23, 2021 11:55
@dlozeve
Copy link
Contributor Author

dlozeve commented Oct 23, 2021

It took a while to fix caching issues, but now it should work: first run without cache, second run with cache.

Gambit master still doesn't compile, I haven't been able to find the source of the error. If someone has an idea, I'm interested! The latest Gambit CI is successful, so this one should compile correctly as well...

Copy link
Collaborator

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

Thank you, this is great!

@vyzo
Copy link
Collaborator

vyzo commented Oct 23, 2021

Let's run it to see the build failure for gambit head.

@vyzo
Copy link
Collaborator

vyzo commented Oct 23, 2021

ok, this looks like we might not setting up the environment correctly -- can we look at gambit's CI and see how they build there?

@dlozeve
Copy link
Contributor Author

dlozeve commented Oct 23, 2021

Actually, I can reproduce the error locally. With a fresh clone of gambit, on master:

./configure --enable-single-host
make

I get the same error as I get in the CI:

_kernel.c: In function ‘___H___kernel’:
_kernel.c:5046:44: error: ‘struct ___virtual_machine_state_struct’ has no member named ‘main_module_id’; did you mean ‘main_module_ref’?
 5046 | ___RESULT = ___VMSTATE_FROM_PSTATE(___ps)->main_module_id;
      |                                            ^~~~~~~~~~~~~~
      |                                            main_module_ref

I don't see anything in their CI config that is significantly different, so I'm not sure where we could go from here. Maybe there is something to do that isn't yet documented in the INSTALL file?

@vyzo
Copy link
Collaborator

vyzo commented Oct 23, 2021 via email

@dlozeve dlozeve force-pushed the github-actions branch 3 times, most recently from 55f6045 to a3d3406 Compare October 24, 2021 15:18
Remove first the files created by the bootstrapping process
@vyzo
Copy link
Collaborator

vyzo commented Oct 24, 2021

might need make bootclean?

@dlozeve
Copy link
Contributor Author

dlozeve commented Oct 24, 2021

I fixed the issue! Turns out make bootclean is not required, Gambit correctly identifies when it needs to bootstrap. However, in the default configuration, the "checkout" GitHub Action does not fetch all branches and tags, and changing that fixed the issue. (I don't know exactly how Gambit's build process depends on other branches and tags, but they do set this explicit setting in their own CI.)

Gambit master builds fine now, but Gerbil's tests do not pass on it.

@vyzo
Copy link
Collaborator

vyzo commented Oct 24, 2021

Great! Dont worry about the test failure in increment, that test is faulty. We can fix it either in this pr (if you want) or a subsequent one.

@dlozeve
Copy link
Contributor Author

dlozeve commented Oct 24, 2021

As you wish!

@vyzo
Copy link
Collaborator

vyzo commented Oct 24, 2021

if you dont mind fixing it, lets get a green checkmark. Basically the expectation of void result from multi increment is incorrect, it is unspecified. So to fix, remove the check around the increment there -- the result is checked below.

@dlozeve
Copy link
Contributor Author

dlozeve commented Oct 24, 2021

Maybe this should also close #80?

@vyzo
Copy link
Collaborator

vyzo commented Oct 24, 2021

ah yes, indeed! Mind updating the description to autoclose?

Copy link
Collaborator

@vyzo vyzo left a comment

Choose a reason for hiding this comment

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

Thank you, this is great!

@vyzo vyzo merged commit 0914bfe into mighty-gerbils:master Oct 24, 2021
@dlozeve dlozeve deleted the github-actions branch October 24, 2021 17:45
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.

migrate CI to github actions Travis should build with gambit master
2 participants