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 Release Builds #691

Closed
wants to merge 15 commits into from
Closed

Conversation

Mightyjo
Copy link
Contributor

@Mightyjo Mightyjo commented Jan 5, 2025

Split from PR #480 here to isolate work on just producing nightly tarballs whenever changes are committed to the default branch.

It's working reasonably well.

Occasionally fails to produce a tarball when the build images for Ubuntu are being updated. Re-running the job the next day usually succeeds. Looks like it's an interaction between the VM images and APT. I can't resolve it without packing my own build images. Plus it's not a big deal.

Refactors the Github Action scripts so they use each others' outputs and don't rebuild from fresh checkouts unless they must. I've done my best to follow current docs on coding these, but I'm looking forward to learning from reviews by y'all how have taught me so much before!

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

Hold this for a minute. Had a problem pop up with the CodeQL job.

Note: I also want to squash down some of the commits like "Shuffle toward insanity" before merging, please.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

Okay, the CodeQL script is good to go. Github's Ubuntu runner removed some of the requirements like I mentioned above. I put the Apt step back to ensure CodeQL finds everything it needs.

(NB: CodeQL's init step was doing dependency discovery and installation a few months ago. I guess they removed the feature.)

@westes
Copy link
Owner

westes commented Jan 6, 2025

Do we need to rename the codeql file? Even if we do, do we need to do that in this pr?

@westes
Copy link
Owner

westes commented Jan 6, 2025

We should bootstrap flex. So, instead of installing flex, from apt, we should build flex from the latest release tarball, then build nightly from that flex.

@westes
Copy link
Owner

westes commented Jan 6, 2025

We should use Ubuntu 22.04 at least. I haven't checked to see what autotools 24.04 comes with, but if they're not going to break flex, it'd be worth the switch to that.

@westes
Copy link
Owner

westes commented Jan 6, 2025

you use tags like "@v4" but the best practice is to use the git hash with the version number as a comment. I -think- the security jobs will pick this up and submit pr's if you don't have a chance to correct it. (Albeit they'll onlyi pick up on it after it's merged.)

@westes
Copy link
Owner

westes commented Jan 6, 2025

If you'll split off the mkskel.sh change, I can merge that immediately. That would be useful in and of itself and it's not atomic with the nightlyi build stuff.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

Do we need to rename the codeql file? Even if we do, do we need to do that in this pr?

The Github wizard regenerated the CodeQL file with a new name while I was fighting with it. I think I can change it back without trouble.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

you use tags like "@v4" but the best practice is to use the git hash with the version number as a comment. I -think- the security jobs will pick this up and submit pr's if you don't have a chance to correct it. (Albeit they'll onlyi pick up on it after it's merged.)

Sorry, thought I caught and updated all of those. Working on it.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

We should use Ubuntu 22.04 at least. I haven't checked to see what autotools 24.04 comes with, but if they're not going to break flex, it'd be worth the switch to that.

Agreed. I mean to be using ubuntu:latest which is currently 22.04 LTS and about to update to 24.04. I'll double check.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

We should bootstrap flex. So, instead of installing flex, from apt, we should build flex from the latest release tarball, then build nightly from that flex.

The build process bootstraps flex from the sources at the tip of the repository. Is that not sufficient?

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

If you'll split off the mkskel.sh change, I can merge that immediately. That would be useful in and of itself and it's not atomic with the nightlyi build stuff.

Sure. Incoming.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

If you'll split off the mkskel.sh change, I can merge that immediately. That would be useful in and of itself and it's not atomic with the nightlyi build stuff.

Sure. Incoming.

See PR #692

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

Updated the build image version and fixed the action tags.

If you merge #692 first, I'll probably need to rebase this branch. Not a problem.

@westes
Copy link
Owner

westes commented Jan 6, 2025

The build process bootstraps flex from the sources at the tip of the repository. Is that not sufficient?

Ah. I noticed that flex is one of the installed packages. It should not be installed from apt since we can build from the release tar ball.

@westes
Copy link
Owner

westes commented Jan 6, 2025

#692 merged; thanks for that.

@westes
Copy link
Owner

westes commented Jan 6, 2025

Agreed. I mean to be using ubuntu:latest which is currently 22.04 LTS and about to update to 24.04.

I'd rather specify 22.04 specifically.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025 via email

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025 via email

@westes
Copy link
Owner

westes commented Jan 6, 2025

download the latest release
tarball. Any chance I can do that as a separate PR?

I adore small orthogonal pr's. release/latest is what you want, according to a quick search-engine-fu.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

Squashed out commits chains that ended up getting reverted.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

download the latest release tarball. Any chance I can do that as a separate PR?

I adore small orthogonal pr's. release/latest is what you want, according to a quick search-engine-fu.

Thanks for googling that for me, lol!

Right now, "releases/tag/nightlies" will become the target of "releases/latest" each time this job runs successfully. I need to turn that behavior off or find a query that gets around it.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

Huh. The nightly.yml is throwing an error. Hang on.

Edit: No idea why nightly.yml is 1) trying to run on PR, 2) failing at line 11 in the PR run. Sorry for the noise in your inbox.

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 6, 2025

Good gravy. Sorry about that. I lost a space at the beginning of nightly.yml:83 and it blew it all up.

I'm going to squash the commits I had to do to track it down.

@westes
Copy link
Owner

westes commented Jan 7, 2025

Installing bison is okay; we'll need it to process our own parser, in fact. There's still an installation of flex it looks like, too.

Fix build image version numbers.
@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 7, 2025

download the latest release tarball. Any chance I can do that as a separate PR?

I adore small orthogonal pr's. release/latest is what you want, according to a quick search-engine-fu.

Installing bison is okay; we'll need it to process our own parser, in fact.

I'm asking to merge this PR now and find a way to install the last release tarball into the Github build image later.

The build system already bootstraps Flex from the current working copy, HEAD in this case, so it doesn't matter which version of lex is installed on the system (as long as it's a POSIX lex).

Flex 2.6.4 and Bison (something recent) are already installed on the Github build images. They didn't need to be in the 'apt install' commands. I just copied it from my build system to be sure I'd gotten everything.

@westes
Copy link
Owner

westes commented Jan 7, 2025

What are the references to swift doing? If it's some github actions thing, just tell me what doc/s I should read up on.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 7, 2025

What are the references to swift doing? If it's some github actions thing, just tell me what doc/s I should read up on.

It was something the Github wizard for CodeQL produced. I can remove it.

Lower CodeQL timeout to 240 minutes.
@westes
Copy link
Owner

westes commented Jan 8, 2025

Thanks for this; I'm rebasing now. I am adding bison bak in since we specifically want it and can't necessarily depend on whatever image having it. More future-proofing things than anything else. I'm also dropping it down to one commit.

@Mightyjo
Copy link
Contributor Author

Mightyjo commented Jan 8, 2025

Thanks for this; I'm rebasing now. I am adding bison bak in since we specifically want it and can't necessarily depend on whatever image having it. More future-proofing things than anything else. I'm also dropping it down to one commit.

I'm happy to make those changes. Give me two shakes.

@westes
Copy link
Owner

westes commented Jan 8, 2025

This is now merged into master.

@westes westes closed this Jan 8, 2025
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.

2 participants