-
Notifications
You must be signed in to change notification settings - Fork 50
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
[ci] GH actions for windows, linux and osx #46
Conversation
I don't deserve to be the only "author" of this commit. |
tell git, I did nothing special ;-) |
What I meant is that you can |
BTW, you may also want to add scheduled pipelines. See coq-community/templates#77 for hints on how to do so. |
Yes! I plan to do that for the other PR, the one using Coq's overlays |
I'm stuck here (on windows):
@MSoegtropIMC if this rings a bell, I'm all ears. (I think we saw that already, but I can't remember what the problem was) |
Thanks, I'll try that! |
@MSoegtropIMC I tried a workaround, see last commit, but does not seem to cut it. I'll try to reproduce it locally after lunch (the only difference I see is that here I don't use an administrative cygwin...) |
I frequently also test to download the zip and run the batch file from a dos console. This did work. Can you check if the paths in the parameter overview dump look correct? |
They look ok, with double \ (but for the cache, which I did not pass explicitly)
|
I cannot reproduce it locally, so I'm sneaking in --verbose and see how it goes. |
One more thought: I think I might have one more bug: the ARCH variable is I think redefined in an inconsistent way somewhere (from / to 32/64 to i686/x86_64) and having ARCH defined before you start the batch file can lead to unwanted effects. |
@gares : please ping me when a build is finished before you change something, so that I can inspect the logs. |
Old logs are available, click on the red |
It failed again, we now have a little backtrace from ocaml
|
@gares : I think it is going wild long before. E.g. in the script before Maybe it ends up in a bad switch by some magic. I would insert some |
From |
@gares: I think there is one depext.ml from depext-cygwin and one from depext plain. |
Btw.: in order to get this working for now you can simply disable depext on cygwin - it should not be required, I add all packages to the cygwin setup. I just added it for adding depext for additional opam packages by the user later. But I would really like to track this down - looks like something which could happen to users as well. Also: can you add a |
I see, the hypothesis is that |
I am not 100% sure if this not normal. I want to compare the logs with a local build, but I am in meetings until one hour from now. |
OK, no problem. Also feel free to push on this branch any commit you like adding prints. I'm going to play with my solder tonight, not with CI ;-) |
@gares : I compared it with the log from a local build and it is pretty much identical up the point it dies. Need to sleep over this ... |
@gares : I have a few ideas what this might be - let me do a few tests. I will ping you when I am through with my ideas. |
@gares : one question: why do you install opam manually on Ubuntu and MacOS? The script should do thsi automatically. Maybe on MacOS this can be improved (it does it via Homebrew or MacPorts and fails if neither is there) but on Ubuntu this does work. |
You suggested to do so (do u recall I wanted to add REGTEST on unix since the opam installer asks questions). |
OK, I'll give it a try. The doc is not super clear about wildcards and windows paths: https://docs.github.com/en/free-pro-team@latest/actions/guides/storing-workflow-data-as-artifacts BTW, it is possible to pass artifacts to other jobs (see the last example in the doc above). Maybe we can also try to run the installer by passing |
Sorry I don't know - I think you know more about CI than I do (and I am very grateful that you went through this effort!)
It should work with "installer /S /D=dir", but it needs to be run from an admin console to avoid the rights elevation question. |
Quite bizarre this depext issue, btw. Not sure why it works in local builds if one is installed after the other. But then depext-cygwinports is version 0.0.7, so I guess there is nobody to blame. But I wonder if I should include it. As I said, I don't need it for anything which is in the platform - I install all packages upfront. This is mostly for the convenience of users who want to install additional packages. |
@Zimmi48 : a few more notes on MSYS2 and Cygwin:
|
After miserably failing to find a way to start a
|
I should have told you that this is my guess. It is rather tricky to do anything unattended otherwise. |
The silly broken log feed spoke: |
I can't see the log yet, but I think we can conclude that the depext issue is solved, so I give this back to you. I will take care of finishing the installer creator (it has a few bugs, stuff like style files are missing and the like). Half done. |
The log thing is buggy, now I see nothing, but a few minutes back it did spit that line, so I guess it's working (and crunching on vst) I'm back on track, thanks! |
I downloaded the zip with logs and I see the "FINISHED" line. It failed on the wrong installer creation command now.
I say thank you! Without your effort it would have taken another few months until we would have had CI for the Coq platform. |
@gares : FYI: I am almost finished with a script to produce a set of smoke test files (one or two for each installed library / plugin) and batch and shell scripts to run them through coqc. This can be later used to extend your smoke test, so no need to work on that. |
OK. The last commit here separates the windows job into two parts, the first one build the installer, the second one downloads it and runs silly tests. The question is: do you need cygwin to run the tests? Because artifacts are max 2G (per repository if I get it right) so I can't put the entire cygwin inside the artifact. We can go back to 1 job (time limit is fine) or rebuild a fresh environment for smoke testing. EDIT: I now see you say |
I need a shell and opam to create the smoke test kit, but it is very small and can be uploaded together with the installer or even be installed (would make sense). Running the smoke test kit just needs what has been installed. |
2fc9d72
to
cf9c2a9
Compare
shell_scripts/install_opam.sh
Outdated
ask_user_opt1_cancel "Disable sandbox (d) or cancel (c)?" dD "dsiable sandbox" | ||
if [[ "${COQREGTESTING:-n}" == n ]] | ||
then | ||
ask_user_opt1_cancel "Disable sandbox (d) or cancel (c)?" dD "dsiable sandbox" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is dsiable a typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh eh, yes
ceb441e
to
2c34167
Compare
I've extracted from #44 the commit adding CI to the platform, based on work by @Zimmi48. Two things before merge:
bad file descriptor
problem before)This commit should work just fine on the v8.12 branch