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

Robust makefile and workflow to build dev and production ASM and WASM versions #14

Merged
merged 72 commits into from
Dec 8, 2022

Conversation

Jaifroid
Copy link
Collaborator

@Jaifroid Jaifroid commented Nov 23, 2022

This is for updates to the workflow I created in #13.

@Jaifroid Jaifroid added the build Code relating to building or publishing assets label Nov 23, 2022
@Jaifroid Jaifroid self-assigned this Nov 23, 2022
@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Nov 23, 2022

This workflow ran into the same issue as documented in #9 (comment), i.e.:

wasm-ld: error: /src/build/lib/libzim.a: archive has no index; run ranlib to add one
wasm-ld: error: /src/build/lib/libzstd.a: archive has no index; run ranlib to add one 

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 libzim.a and libzstd.a not having indices? According to the suggested solutions I found, it should be something very small, but I don't know where to add one of those commandline parameters.

@Jaifroid Jaifroid added the help wanted Extra attention is needed label Nov 23, 2022
@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Nov 24, 2022

Success! Adding ar = 'emar' to emscripten-crosscompile.ini allows the build to succeed.

I now need to add some steps to save the built files a.out.js and a.out.wasm. @kelson42, what would you recommend? Should the built files be pushed to master (they are on master), to a branch/PR, or added as an asset to a draft release (which would only be accessible to maintainers of this Repo until published)?

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Nov 24, 2022

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:

  • Decide if this workflow should run automatically on push to master, or only be manually dispatched
  • Decide if this workflow should update master with the newly built files, or create a PR to do so, or merely archive the artefacts as at present
  • Create a similar workflow to run nightly, but using the libzim.a from kiwix-build (Use the libzim.a generated by kiwix-build #12) and publishing to download.kiwix.org
  • Decide which workflow should run when creating a release/tag and publishing a.out.js and a.out.wasm to download.kiwix.org/release
  • Add some automatic tests to ensure wasm is working.

@Jaifroid Jaifroid requested a review from kelson42 November 24, 2022 18:35
@Jaifroid
Copy link
Collaborator Author

@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:

  • Is the workflow correctly named (Docker Inage CI)?
  • Should it be run on push to master?
  • What should I do with the build artefacts (they are currently archived under the workflow run)?

@kelson42
Copy link

@Jaifroid I will have a look this WE.

@kelson42
Copy link

  • s the workflow correctly named (Docker Inage CI)?

I prefer to just call it CI

  • Should it be run on push to master?

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 master is OK. Like here
https://github.com/openzim/mwoffliner/blob/master/.github/workflows/ci.yml#L3

  • What should I do with the build artefacts (they are currently archived under the workflow run)?

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.

@Jaifroid
Copy link
Collaborator Author

I prefer to just call it CI

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 (libzim.a). I think mgautierfr will complete that integration, if I've understood correctly.

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 master is OK. Like here https://github.com/openzim/mwoffliner/blob/master/.github/workflows/ci.yml#L3

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.

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.

Thanks, I'll take a look at that.

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.

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 libzim.a from kiwix-build?

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.

@Jaifroid
Copy link
Collaborator Author

Here's what I propose:

  • This workflow builds the libzim WASM based on the latest released version of libzim and a known stable version of Emscripten (as provided in their Docker container). As such, it could be run when a tag is created, and it could upload its output to GitHub releases (also, potentially to download.kiwix.org/release/). It can also be run on-demand (workflow dispatch), in which case it would not upload its output, only archive it under the workflow. When we have tests, it would also run tests on what it has built.
  • A different CI would be run nightly and on pull request / push to master. The nightly CI would take the latest nightly version of libzim.a, build the libzim WASM and the Web Worker, and then test them. It could also use the latest version of Emscripten, not a Docker container. If it's a nightly job (rather than push/PR), it would also upload its output to download.kiwix.org/nightly/.

Does this sound like a useful distinction? The main difference is that the input for the nightly CI would be the potentially unstable, nightly libzim.a, rather than the released version.

@kelson42
Copy link

I prefer to just call it CI

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 (libzim.a). I think mgautierfr will complete that integration, if I've understood correctly.

@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

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 master is OK. Like here https://github.com/openzim/mwoffliner/blob/master/.github/workflows/ci.yml#L3

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.

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.

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.

Thanks, I'll take a look at that.

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.

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 libzim.a from kiwix-build?

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.

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.

Yes, I understand, if this has to be done that way, why not.... but pleast don't compile the libzim here.

@kelson42
Copy link

Sorry, if there is a kind of redundancy with my previous comment.

* This workflow builds the libzim WASM based on the latest _**released**_ version of libzim and a known stable version of Emscripten (as provided in their Docker container). As such, it could be run when a tag is created, and it could upload its output to GitHub releases (also, potentially to download.kiwix.org/release/). It can also be run on-demand (workflow dispatch), in which case it would not upload its output, only archive it under the workflow. When we have tests, it would also run tests on what it has built.

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.

* A different CI would be run nightly and on pull request / push to master. The nightly CI would take the latest _**nightly**_ version of `libzim.a`, build the libzim WASM and the Web Worker, and then test them. It could also use the latest version of Emscripten, not a Docker container. If it's a nightly job (rather than push/PR), it would also upload its output to download.kiwix.org/nightly/.

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.

Does this sound like a useful distinction? The main difference is that the input for the nightly CI would be the potentially unstable, nightly libzim.a, rather than the released version.

From a high level, yes.

@Jaifroid
Copy link
Collaborator Author

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 libzim.a as well as nightly versions that the scripts here can work off.

@Jaifroid Jaifroid changed the title Update workflow to build WASM Robust makefile and workflow to build dev and production ASM and WASM versions Dec 1, 2022
@Jaifroid Jaifroid marked this pull request as draft December 1, 2022 22:44
@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Dec 3, 2022

@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?

  • Where should these released files be uploaded? They are "drivers" rather than complete working applications. They will be released in GitHub releases -- see automatically created draft release here -- but I also need to know where to put them on download.kiwix.org.
  • I don't have access to this Repo's Settings / Secrets -- I need to add the secret to access master.download.kiwix.org in GH Actions. Could you either give me enough access rights, or else tell me the name of the secrets (if they have been added as org secrets)?
  • What file format should I use? There is no "architecture", but I guess we could consider asm and wasm as pseudo-architectures, so I would simply propose:
    • libzim_wasm_0.0.0.zip
    • libzim_asm_0.0.0.zip

Many thanks.

@kelson42
Copy link

kelson42 commented Dec 3, 2022

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?

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Dec 3, 2022

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?

@kelson42 The logic of this workflow is such that it will be easy to plug in the libzim.a and its dependencies once we have working versions. They are currently failing -- see my report in kiwix/kiwix-build#552 (comment). I don't know how to fix that on the kiwix-build side. This workflow has the capacity to build everything from scratch (a capacity we should retain, so as not to lose the know-how for doing it with Emscripten), but it will be easy to make it use pre-built binaries instead, or to provide a workflow dispatch option to use either for testing, defaulting to prebuilt.

Otherwise, it should be published in https://download.openzim.org/release/javascript-libzim IMO.

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.

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?

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Dec 4, 2022

All tasks mentioned in #14 (comment) are completed and everything is compiling successfully.

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Dec 6, 2022

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:

  • Pushed a tag 'v.0.0.1' (on this PR/branch) to origin (this can also be done from the GitHub Web UI, Workflow Dispatch for the Action);
  • Action automatically created a draft release on GitHub and added compiled assets;
  • After checking, I marked draft release as preview and published (manually from the GitHub UI);
  • Another action automatically downloaded the published assets and uploaded them to https://download.openzim.org/release/javascript-libzim/

image

If I can merge this PR, then I can start work on a nightly version that will use libzim.a etc. from kiwix-build. Although that version will be currently broken, there is nothing to stop me setting up the workflow. Once the workflow is available to use kiwix-build and the binaries have been fixed (openzim/libzim#751), then I can switch this workflow to using libzim.a, and/or make building from scratch an option.

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Dec 6, 2022

@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:

  • Add a conditional step after checkout that downloads the latest nightly release of kiwix-build WASM binaries, and unzips them to the correct locations;
  • Run the makefile, which will not rebuild these binaries, but will use them to compile the WASM/ASM;
  • Archive these versions under the Workflow, and publish to download.openzim.org/nightly/...

@Jaifroid
Copy link
Collaborator Author

Jaifroid commented Dec 8, 2022

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).

@Jaifroid Jaifroid merged commit 27e4ef7 into master Dec 8, 2022
@Jaifroid Jaifroid deleted the Create-workflow-to-build-WASM branch December 8, 2022 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Code relating to building or publishing assets
Projects
None yet
2 participants