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

nightly: Use GitHub Actions for scheduled builds #3334

Merged
merged 3 commits into from
Jun 15, 2024

Conversation

JoeKar
Copy link
Collaborator

@JoeKar JoeKar commented Jun 9, 2024

Hopefully this is a more robust approach to schedule, build and upload the nightlies.

  • cross-compile.sh improved for the GitHub Action
  • cross-compile.sh includes MD5 sums for artifact verification

Fixes #3332

@Neko-Box-Coder
Thank you for the input.

@Gavin-Holt
Copy link

Hi,

I am not very familiar with GitHub Actions.

  • Will releases made with GitHub Actions, be available to users without a GIT login?
  • Will they be easy to find for those not familiar with GIT?
  • Could the download link from https://micro-editor.github.io/ be made to work?

I am concerned that this great project remains as easy to access as possible, and that there is nothing off-putting to new users:

  • Loss of consistency between merged PRs and binary releases.
  • Download links pointing to Oct23 release.
  • Login barrier to download binaries.

Hopefully we can get over this bump in the road, and continue enjoying Micro.

Kind Regards Gavin Holt

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 9, 2024

  • Will releases made with GitHub Actions, be available to users without a GIT login?

Yes, but here we're discussing the nightlies, not the releases.

  • Will they be easy to find for those not familiar with GIT?

Maybe they're harder to find for people expecting them under "releases" resp. https://github.com/zyedidia/micro/releases?q=nightlies.
Nightlies shouldn't be used by folks expecting "stable" builds/releases, because they can be somehow broken or introduce regressions even when they shouldn't.

From my perspective it still works, because it points to the last available release, which currently is the v2.0.13.

I am concerned that this great project remains as easy to access as possible, and that there is nothing off-putting to new users:

  • Loss of consistency between merged PRs and binary releases.

Yes, indeed this will introduce some inconsistency where new nightlies can be found, but unfortunately we don't have access to every single bit of micro and his processes and it isn't the first time the nightly step is stuck.

  • Download links pointing to Oct23 release.

Yes, see above, because it's the last real release.

  • Login barrier to download binaries.

That is indeed a valid point. Only the "real" release artifacts can be accessed without login. :(

Copy link
Contributor

@Neko-Box-Coder Neko-Box-Coder left a comment

Choose a reason for hiding this comment

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

The README (under eget) is using nightly tag. We should put a warning above it

Could we also mention this in README so that people know where to go?

@@ -0,0 +1,36 @@
name: Nightly builds
on:
schedule:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
schedule:
workflow_dispatch: # Allows manual trigger
push:
branches:
- master

I feel like triggering every night is a bit too much, and it spams the Actions page

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Look here how the actions log is already "spammed" by every automated "Build and Test" trigger for approved (for building) PR.
In my opinion one build per day more doesn't make the difference here.

Wikipedia supports my understanding of what a nightly is used for. 😉

Does a nightly really need to be started manually? We should keep it simple and trigger it once a day for the actual present HEAD of the master.

The nightly tag could become handy in the moment we like to go the way to create a kind of nightly "release" to publish it globally (no GitHub account needed for the download). But then the nightly tag must be set automatically and a third party action must be used for the release handling.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mind either way, it's just a suggestion and how I normally do it.

@Neko-Box-Coder
Copy link
Contributor

Neko-Box-Coder commented Jun 10, 2024

Should we try to use github action to modify the release page?
Maybe github action has permission to do that.

https://github.com/marketplace/actions/automatic-releases

@Gavin-Holt
Copy link

Hi

Firstly, apologies for confusing Nightlies with Releases I do appreciate the differences.

Secondly, apologies but I can't find any new binaries in the Actions section of this repository. I can see the build and test that @JoeKar ran successfully yesterday. I believe I am looking for Artifacts but can't find any.

I am keen to try a windows binary from the current master branch - incorporating all the hard work merged in since April.

Any guidance?

Kind Regards Gavin Holt

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 11, 2024

I believe I am looking for Artifacts but can't find any.

They will be available first in the moment the PR and thus .github/workflows/nightly.yaml is merged to master, otherwise the new action isn't considered yet.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 11, 2024

Should we try to use github action to modify the release page? Maybe github action has permission to do that.

https://github.com/marketplace/actions/automatic-releases

We could, but I've doubts that this will work, since it requires the ${{ secrets.GITHUB_TOKEN }} and I'm unsure if it really is stored and valid. As far as I understood the scripts under tools/ ... hub was used, which is a local git extension to interact with GutHub and most probably the local credentials.

@Neko-Box-Coder
Copy link
Contributor

We could, but I've doubts that this will work

I am inclined to try it. ${{ secrets.GITHUB_TOKEN }} comes with Action by default, you don't need to configure anything (apart from having github action on ofc).

You can read more about it here

As for permissions, the release page falls under contents category from the PAT/API access which github action does seem to have write permission by default. See here

I don't think it will hurt anything giving it a try.

If it works, it will solve the long-standing "we can't create releases" issue and don't require people to go to the action page (and have github account) to get the binaries.

If it doesn't, we can just go back to the plan discussed in this PR.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 11, 2024

Ok, I see. Thank you for the poke into the docs.
Then I will try to give it a shot inside this PR, we can agree to the details and see if it works with a nightly build first.

In case everything goes fine we can try to move the release creation into a manual and/or release tag (v#.#.#) triggered build too.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 12, 2024

In my fork it was already working, but I had to activate Read and write permissions for the action Workflow permissions inside my forks repository settings, otherwise a tag can't be pushed.
Currently I can't check this setting, since my rights aren't sufficient enough. I assume, that the actions settings stay at defaults with read permissions only.

Edit:
Existing artifacts for the same release will be removed by the 3rd-party package ( 👍 ), before the next will be uploaded. This should prevent unneeded storage leaking.

@Neko-Box-Coder
Copy link
Contributor

Neko-Box-Coder commented Jun 12, 2024

@JoeKar
Okay yeah, the default was read/write for github action until Feb 2023. So by default now it is read-only unless changed by the owner.
Now I doubt this will work but it's still worth a try if you can push to master?
If not, we can just use github action artifacts for now and forget about release.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 13, 2024

I wrote Zachary a direct mail. Maybe he can support here short noticed.

tools/cross-compile.sh Outdated Show resolved Hide resolved
@zyedidia
Copy link
Owner

Thanks for looking into this. The permissions are already set to read/write under "Workflow permissions." Is there something that is not currently working for the actions?

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 14, 2024

Is there something that is not currently working for the actions?

To be honest, we didn't try so far, since we wouldn't push stuff were we're unsure if it works or not. But this information is awesome, because then we're able to leave this task up to cyclic Actions and we can plan to do the releases on a similar way (just add tags and let react accordingly).

nit: Just wondering, maybe it's better to use .sha256 extension, to make it clear which sum it is?
What is the convention for sha256 sum file extension (if there is any)?

I though about this as well, but couldn't find an "official" convention for the SHA-# file extension. I decided to go with the old-school 3-digit and common extension name, because it doesn't make a difference when we use something >SHA-256 in the future.

This...

shasum -c *.sha

...will check whatever it's in no matter if SHA-1, 3, 256, 512 etc.pp and no one needs to care about it (we don't have to use sha256sum -c *.sha or sha256sum -c *.sha256. Who needs to know what's used can have a look and check the hash length.

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 14, 2024

This...

shasum -c *.sha

...will check whatever it's in no matter if SHA-1, 3, 256, 512 etc.pp and no one needs to care about it (we don't have to use sha256sum -c *.sha or sha256sum -c *.sha256. Who needs to know what's used can have a look and check the hash length.

Ah indeed, good point.

tools/package-deb.sh Outdated Show resolved Hide resolved
cp assets/packaging/micro.1 micro-$1
cp assets/packaging/micro.desktop micro-$1
cp assets/micro-logo-mark.svg micro-$1/micro.svg
#!/bin/sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's a good idea to use #!/bin/sh -e? Any of the commands in this script may potentially fails, so we'd better not ignore such failures silently?

What do these github actions do if the command (cross-compile.sh in this case) exits with a non-zero status? I hope they abort the action and log the stderr output?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

set -e added. The action will stop at the first error and not proceed the upcoming steps. As far as I've seen while testing in my fork it prints the error which appeared.

1. doesn't need a given parameter to set the VERSION,
since it is determined itself
2. moves the *.deb only in case package-deb.sh succeeded
3. rename *.tar.gz to *.tgz shorten the extension to...
4. add SHA256 sums per artifact
Copy link
Collaborator

@dmaluka dmaluka left a comment

Choose a reason for hiding this comment

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

Let's try it.

@JoeKar JoeKar merged commit ced6d94 into zyedidia:master Jun 15, 2024
3 checks passed
@JoeKar JoeKar deleted the feature/action-nightly branch June 15, 2024 12:35
@Gavin-Holt
Copy link

Hi,

I waited up 'til 1 AM (BST), like an expectant father, but no new bouncing baby binary!

Happy Fathers Day

Gavin Holt

C:\Users\PC\Downloads>micro -version
Version: 2.0.14-dev.181
Commit hash: 1f51d0b
Compiled on June 16, 2024

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 16, 2024

@Gavin-Holt:
You took one of the older ones, which still seem to be generated.
The newer ones are called micro-0.0.0-unknown-*. Why ever they don't receive the name interpolated from the last release. 🤔

Edit:
I would be more happy with the short commit has as replacement in the moment no previous release tag could be found.
Currently I assume, that the environment in which the build runs doesn't know the tags.

As expected (with --no-tags)...

/usr/bin/git -c protocol.version=2 fetch --no-tags --prune --no-recurse-submodules --depth=1 origin +refs/heads/master*:refs/remotes/origin/master* +refs/tags/master*:refs/tags/master*

@Gavin-Holt
Copy link

😊 I have it now.

Many thanks to all those who made this delivery possible.
I will away, and test out the new features.

Kind Regards Gavin Holt

@Gavin-Holt
Copy link

Hi

The Nightlies are working now ...

Microsoft Windows [Version 10.0.19044.3086]
(c) Microsoft Corporation. All rights reserved.

C:\Users\PC\Downloads\micro>dir
 Volume in drive C has no label.
 Volume Serial Number is 481B-272E

 Directory of C:\Users\PC\Downloads\micro

24/06/2024  18:34    <DIR>          .
24/06/2024  18:34    <DIR>          ..
24/06/2024  18:33         4,665,929 micro-2.0.14-dev.222-win64.zip
24/06/2024  18:34        11,615,232 micro.exe
               2 File(s)     16,281,161 bytes
               2 Dir(s)  947,525,398,528 bytes free

C:\Users\PC\Downloads\micro>micro.exe --version
Version: 2.0.14-dev.222
Commit hash: 882b98f3
Compiled on June 24, 2024

but the displayed information does not match.

  • Left side has wrong date and correct commit hash
  • Right side has correct date and wrong commit hash

image

Is it very difficult to make the Nightly show information which matches the content of the zip files?

Sorry I have no clue myself, but thought I should highlight the inconsistencies.

Kind Regards Gavin Holt

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jun 25, 2024

This is partly caused by the old builds (micro-2.0.14-dev.181-*) which are still generated by the old nightly approach triggered by @zyedidia. These builds can now be disabled, but unfortunately we've no control over them.

@Neko-Box-Coder
Copy link
Contributor

Neko-Box-Coder commented Jun 30, 2024

@JoeKar
Very nice that nightly works now but @zyedidia CI builds and our Github CI builds are quite confusing to anyone not reading this thread.

Could you email @zyedidia again for disabling his CI build for micro, since you have emailed him already on this topic.

Thanks.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Jul 21, 2024

Could you email @zyedidia again for disabling his CI build for micro, since you have emailed him already on this topic.

Did it today.

@zyedidia
Copy link
Owner

Thanks for setting up GitHub Actions to release nightly builds! I have disabled the old method.

Comment on lines +23 to +25
tar -czf micro-$VERSION-$1.tgz micro-$VERSION
sha256sum micro-$VERSION-$1.tgz > micro-$VERSION-$1.tgz.sha
mv micro-$VERSION-$1.* binaries
Copy link
Collaborator Author

@JoeKar JoeKar Aug 5, 2024

Choose a reason for hiding this comment

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

I realized using $VERSION within the archive resp. asset name is counterproductive when used for the nightly upload, because the softprops/action-gh-release will only delete and update assets with the same name present.
Having an updated from e.g. 2.0.14-dev.243 -> 2.0.14-dev.244 -> 2.0.14-dev.245 fills the nightly "release" with all of the assets.
The correction of the nightlies currently needs manual adjustment to be cleaned up.

So I suggest to slightly adapt the tools/cross-compile.sh again to allow a custom string as archive version, if given otherwise use the parsed version. The nightly action then can use tools/cross-compile.sh nightly to provide fixed asset names, which will be properly handled by the softprops/action-gh-release action.
I wasn't able to find an option cleaning up all the assets present at one release.

Side note:
The SHA sum created for the nightlies is only valid till the next build will be done for the same version.

PR: #3418

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.

Nightly builds are not the current master branch 😭
5 participants