-
Notifications
You must be signed in to change notification settings - Fork 1
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
Robust makefile and workflow to build dev and production ASM and WASM versions #14
Conversation
This workflow ran into the same issue as documented in #9 (comment), i.e.:
You can see the error in the workflow log here: https://github.com/openzim/javascript-libzim/actions/runs/3535606139/jobs/5933803167#step:4:9482. It's very strange that mossroy was able to compile this way, but I could not reproduce a successful compile via docker running on WSL, and now GitHub actions running on real Ubuntu-latest cannot reproduce a successful compile either. @mgautierfr Do you have any clue how to solve this error with |
Success! Adding I now need to add some steps to save the built files |
For now, I've added steps to archive the artefacts under the action. You can see an example here (scroll to bottom of page, can be downloaded as a zipped archive): https://github.com/openzim/javascript-libzim/actions/runs/3539162287 Next steps would be:
|
@kelson42 I've asked for you to review this PR, but really it's the whole workflow on this branch I'd like advice on, and these points in particular:
|
@Jaifroid I will have a look this WE. |
I prefer to just call it
Usually I want to have in it in pull requests (for obvious reasons) and each time a commit happens on master (to be able to have an overall CI badge and know if
We have different approaches over repositories, but I wonder if https://github.com/openzim/mwoffliner/blob/master/.github/workflows/ci.yml#L3 should not be the ultimate solution. Hopefully I understand what you do properly. What I wonder here, is that all run in a Docker instance so this is pretty not transparent for me. Not sure we need that anymore. or even we should do that. On the top of it, it seems this Docker compiles everything, but here the binaries done by libzim CD should be used. |
That would be fine. I'm not sure if this should be the nightly CI, however, because this builds libzim from scratch, and doesn't use the output of kiwix-build (
So I guess it depends on the status of this workflow: I conceived it just as a tool to be run manually when a dev wants to build everything from scratch on-demand.
Thanks, I'll take a look at that.
Emscripten provides Docker images for developers to run it in a consistent and controlled environment. It's convenient, but of course we could provide an Emscripten setup in a GitHub Ubuntu instance. Maybe @mgautierfr would prefer that we do it that way for a nightly CI that uses To be clear, the Docker container attaches the files from this Repo, and uses them to build the WASM, so it's not quite a black box, and it does provide full logging, as you can see from the latest action run. |
Here's what I propose:
Does this sound like a useful distinction? The main difference is that the input for the nightly CI would be the potentially unstable, nightly |
@mgautierfr might help to understand why you can not use the libzim release wasm code, but not to rebuild the recompile the libzim. This should not take here in this repo, at least for sure not in the CI. Duties of respective repositories should not be mixed. The nightly build should not be part of the CI, but of the CD (this should be an other workflow). Here maybe something to read https://www.atlassian.com/continuous-delivery/principles/continuous-integration-vs-delivery-vs-deployment
There seems to be a fundamental mix between this Docker image/container which was useful at some point but for which I see no usage anymore and what should be a CI. The CI is an automated process to secure no regression are introduced with new code. So here the CI of this repository should test the code of this repo. As such it is an automatic tool applied on relevant branches, typically feature branches or master.
Emscripten provides Docker images for developers to run it in a consistent and controlled environment. It's convenient, but of course we could provide an Emscripten setup in a GitHub Ubuntu instance. Maybe @mgautierfr would prefer that we do it that way for a nightly CI that uses This might be convenient, but that does not mean we should or have to use it for the CI and it does not mean either we should compile the libzim here. Yes, we insist that this is not the purpose of this repository to compile the lizim, but only the wrapper code.
Yes, I understand, if this has to be done that way, why not.... but pleast don't compile the libzim here. |
Sorry, if there is a kind of redundancy with my previous comment.
OK. Usually our CD workflows deploy source code to Github releases and download.kiwix.org/releases, but push the binary builds (here wasm) to download.kiwix.org/releases. I think this is still the right approach.
Yes, this is how it works usually. You have to understand that by doing so, you will have to pretty quickly implement libzim changes in the wrapper. I recommend otherwise to use a fix/stable versions of dependences and compilation tools.
From a high level, yes. |
OK, thanks for the useful insights, @kelson42. I'm very new to CI and CD. And I'm very happy not to build libzim here once we have public, released versions of |
@kelson42 I'm about to add code to the workflow that will upload the zipped WASM and ASM files, including wrappers, to master.download.kiwix.org. These would be "release" versions (stable) rather than nightly, as discussed above. Could I ask you to decide the following?
Many thanks. |
I would really prefer to push releases based on libzim of http://download.openzim.org/release/libzim. Fixing #8 should be the priority, @mgautierfr What do you think? Otherwise it should be published in https://download.openzim.org/release/javascript-libzim IMO. Format of your release files looks good IMO, @rgaudin ? @rgaudin Could you please push here the necessary secrets so we can upload to https://download.openzim.org/release/javascript-libzim from this repo? |
@kelson42 The logic of this workflow is such that it will be easy to plug in the
OK, thanks. Until #12 / kiwix/kiwix-build#552 can be fixed, I'd like to publish a version 0.0.1 that I could use for kiwix/kiwix-js#935 (which is working well). It's a major advance.
|
All tasks mentioned in #14 (comment) are completed and everything is compiling successfully. |
Now that we have the SSH_KEY secret in this Repo, I just made a pre-release version using this branch as a test of the full cycle. Procedure:
If I can merge this PR, then I can start work on a nightly version that will use |
@kelson42 Would you prefer me to do the nightly build from kiwix-build binaries on top of this PR (it will re-use the workflows here, with conditional branching if it's a chron job)? I'm just aware that this PR is becoming massive, and it might be better to merge it and then build the nightly workflow off master. Procedure is quite simple:
|
I am going to release an updated WikiMed using libzim for full text search. To do so, I need to be able to point to the source code for the libzim release, and to do this, I need to merge this PR and create a v0.0.2 pre-release with a tag that won't disappear when this branch is squashed. Merging this PR also means I can pretty quickly implement #12 to start using the kiwix-build binaries as source for compiling the WASM and ASM versions. So, unless I hear to the contrary, I'll merge this PR later today, Thursday (afternoon). |
This is for updates to the workflow I created in #13.