Setup a CI to push generated bindings to a dedicated branch

[yutannihilation]

Related to #57

This pull request adds a job to gather the uploaded generated bindings and commit them to generated_bindings branch. We can create a pull request from that branch at any point we want to update the bindings. Of course we can commit this to the master branch, but I feel this should be done manually with checking the diffs by our eyes, at least at the moment.

This does nothing if there's no diffs, and even if something goes wrong, this pushes to non-master, where no CI is set up. So, I believe this should be safe.

This pull request adds a scheduled run setting. I use this on https://github.com/ggplot2-exts/gallery for a while and it works fine.

@Ilia-Kosenkov @clauswilke
Please let me know if there's any concern, or any topic to discuss.

[Ilia-Kosenkov]

The idea looks good. What about adding workflow_dispatch: trigger? This will allow us to manually force re-generation without making a PR.

Also, what if we somehow notify that there are new bindings? Is it possible to create an automatic issue in this repository? I am afraid that re-generated bindings will disappear silently in the notifications.

[yutannihilation]

What about adding workflow_dispatch: trigger? This will allow us to manually force re-generation without making a PR.

Thanks, sounds good. Could you explain a bit more about the latter sentence? Do you mean workflow_dispatch triggers a push to master directly? Or, is that about some new mechanism we might introduce in the future?

Also, what if we somehow notify that there are new bindings? Is it possible to create an automatic issue in this repository?

I think it's possible. Should it create a pull request, instead of a issue, when there are new bindings? Regarding notification, it seems it's also possible to notify on Discord: https://github.com/marketplace/actions/actions-for-discord.

[Ilia-Kosenkov]

Thanks, sounds good. Could you explain a bit more about the latter sentence? Do you mean workflow_dispatch triggers a push to master directly?

No no, it is just another trigger. We trigger on PR, you add cron (every 3 days), I suggest to also have workflow_dispatch. That way you can trigger it directly from the Actions menu on GitHub. If there are no downsides to this, I think it is a good addition. Imagine that your scheduled run was yesterday, a new version of R is released today, and we have to wait for 2 days for bindings to re-generate or create a PR/commit to trigger the workflow. With dispatch, we can just start it when we need (in addition to on pull request and cron).

I think it's possible. Should it create a pull request, instead of a issue, when there are new bindings? Regarding notification, it seems it's also possible to notify on Discord: https://github.com/marketplace/actions/actions-for-discord.

I was thinking about the following. If new bindings are generated, then we crate an issue saying: "The generated bindings have changed for these platforms" and some additional information (like diffs?), maybe the version of R and cargo used for generation. That way a maintainer can see there is an issue and decide what to do manually -- update bindings, publish new version to crates.io, ignore, etc.

[yutannihilation]

a PR/commit to trigger the workflow

Ah, thanks for clarifying! Sorry, I didn't explain that this runs only on master and ignores pull requests (so I was a bit confused why pull request is related here but it's totally my fault...). You can see commit_generated_bindings is actually skipped on this pull request.

libR-sys/.github/workflows/test.yml

Lines 281 to 282 in 4bc89ee

	# Do not run this on pull request
	if: github.ref == 'refs/heads/master'

Then I think I get your point. Let's add workflow_dispatch.

[yutannihilation]

If new bindings are generated, then we crate an issue saying: "The generated bindings have changed for these platforms" and some additional information (like diffs?), maybe the version of R and cargo used for generation.

I feel it might be a bit easier to use pull requests to include diffs, but I'll investigate both way.

[yutannihilation]

Here's example issue and pull request:

• issue: Update the precomputed bindings yutannihilation/libR-sys#10
• pull request: Update the precomputed bindings yutannihilation/libR-sys#11

Looking at the actual diffs, I'm afraid this happens too often as there are such information like SVN revisions included. So, I'd stick with no issue or pull request for now. It's probably too complicated to handle programmatically.

[Ilia-Kosenkov]

OK, let's see how it goes.

[clauswilke]

Fine by me. One issue we'll have to think through at some point is what to do with old bindings for older R versions that we're not covering anymore in our CI setup. Whenever a new R version is released, an old one drops out of coverage. Do we delete those bindings? Ideally we should, I think, since otherwise they can get out of sync with other changes in libR-sys.

[yutannihilation]

Do we delete those bindings?

Sorry, I forgot to answer this question on #57. I agree we should. Let's discuss this again on #57.

It's probably too complicated to handle programmatically.

Thinking this again, maybe we can just ignore the devel bindings...? Let's revisit here when we need better mechanism to notify.

[yutannihilation]

Hmm, this fails because bindings-macos-x86_64-R4.1.rs is generated on two macOS runners (I didn't notice this because macOS-11 runner don't run on my fork).

cp: will not overwrite just-created 'bindings/bindings-macos-x86_64-R4.1.rs' with 'generated_binding-macOS-latest-R-release-rust-stable/bindings-macos-x86_64-R4.1.rs'

[yutannihilation]

@Ilia-Kosenkov
Do you remember why macos-11 runner needs to check default target in addition to aarch64-apple-darwin? I briefly checked discussions around #45, but couldn't find the reason. If we can remove this, we can avoid the above conflict.

libR-sys/.github/workflows/test.yml

Lines 38 to 39 in 8fd988b

	- {os: macOS-11, r: 'release', rust-version: 'nightly', targets: ['default', 'aarch64-apple-darwin'],
	no-test-targets: 'aarch64-apple-darwin', emit-bindings: 'true'}

I already added an workaround for this (8fd988b), so this isn't very urgent, but I'd like to know the background.

[Ilia-Kosenkov]

I am not sure, but I think we tested to make sure it works on the next MacOS version. I guess it can be removed. I am sorry that I did not write any comments, at that time we were just trying to add macos11 to get arm bindings.

[yutannihilation]

I see, then I'll create a pull request to update the runner. No worries.