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

Check that tarballs can build #1931

Closed
mhart opened this issue Sep 29, 2019 · 10 comments
Closed

Check that tarballs can build #1931

mhart opened this issue Sep 29, 2019 · 10 comments

Comments

@mhart
Copy link

mhart commented Sep 29, 2019

As per nodejs/node#29712

@MylesBorins
Copy link
Contributor

Adding a note here that I think a slightly better solution would be to create the tarball and then build all release assets from the tarballs. That way what we are building and releasing can better match what downstream folks are building

@mistydemeo
Copy link

mistydemeo commented Mar 4, 2020

Cross-linking nodejs/node#31858 as another example where this happened.

@rvagg
Copy link
Member

rvagg commented Mar 4, 2020

someone needs to script this up, it might be a good one to put into GitHub Actions even, but if not, we have our containerised builds where we can put it, but it needs someone with the time work up a script to do a full "build tarball, unpack tarball, build from unpacked tarball, test " cycle.

@jkleinsc
Copy link

jkleinsc commented Mar 5, 2020

I can take a look at this. Just to verify - this is something that would run on release CI only?

@rvagg
Copy link
Member

rvagg commented Mar 6, 2020

@jkleinsc nope, we should just integrate it into ci.nodejs.org or GitHub Actions for each commit. Make sure that nothing introduced during normal dev breaks tarballs—this has happened numerous times in the past and never gets noticed until a release breaks.

We have some miscellaneous things going through GitHub actions already: https://github.com/nodejs/node/tree/master/.github/workflows this might fit, but also might incur built time timeouts? Traditionally we've put build permutations into https://ci.nodejs.org/job/node-test-commit-linux-containered/ and this would certainly fit there, but we're embedding the scripts directly into Jenkins which isn't great. So for now, the easy path is coming up with a script that takes a clean cloned nodejs/node and steps through a full make-tarball (make tar ...), unpack tarball, build from unpacked tarball, test from binary built from tarball. We can then either drop that script into Jenkins, or put it in this repo and run it from a curl|bash which is a pattern we've been adopting more and more with Jenkins.

@jkleinsc
Copy link

jkleinsc commented Mar 6, 2020

@rvagg ok makes sense. I think I’ll see if I can get it working as a GitHub action.

@richardlau
Copy link
Member

We have some miscellaneous things going through GitHub actions already: https://github.com/nodejs/node/tree/master/.github/workflows this might fit, but also might incur built time timeouts?

Unlikely. GitHub is much more generous over how long a workflow/job can run compared to Travis CI (currently 6 hours per job or 72 hours for a workflow, https://help.github.com/en/actions/getting-started-with-github-actions/about-github-actions#usage-limits, compared to 50 mins for a Travis job).

There was a recently added job to nodejs/node that builds debug and runs ASAN tests that currently takes 3 hours to run.

@jkleinsc
Copy link

jkleinsc commented Mar 6, 2020

nodejs/node#32129 should resolve this issue.

@MylesBorins
Copy link
Contributor

awesome work @jkleinsc left a comment on the issue. A thought I had, we should likely still do this directly in CI with the tarball we plan to ship... not that we should be breaking anything with a release commit, but I'd personally be a bit more comfortable if we had the extra gate.

jkleinsc pushed a commit to jkleinsc/node that referenced this issue Apr 2, 2020
jkleinsc pushed a commit to jkleinsc/node that referenced this issue Apr 9, 2020
@mhart
Copy link
Author

mhart commented Apr 13, 2020

🙌

BethGriggs pushed a commit to nodejs/node that referenced this issue Apr 14, 2020
Fixes: nodejs/build#1931
PR-URL: #32129
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BridgeAR pushed a commit to nodejs/node that referenced this issue Apr 28, 2020
Fixes: nodejs/build#1931
PR-URL: #32129
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants