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

build,test: add .ci.yml for containered tests (WIP proposal) #30057

Closed
wants to merge 6 commits into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Oct 22, 2019

I'm working up a proposal for Build, this can be safely ignored for now, I'll be getting the rest of it up and documented in nodejs/build for further discussion.

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Oct 22, 2019
@rvagg
Copy link
Member Author

rvagg commented Oct 22, 2019

Documentation for this PR is currently @ nodejs/build#1982, this PR serves as part of that proposal. "Show all checks" below in GitHub's status checks shows some of what this is doing.

@rvagg rvagg added the build Issues and PRs related to build files or the CI. label Oct 22, 2019
.ci.yml Outdated Show resolved Hide resolved
@rvagg rvagg force-pushed the rvagg/testing-containers branch 2 times, most recently from 9bbecd5 to a64c93f Compare October 23, 2019 05:13
.ci.yml Outdated Show resolved Hide resolved
@rvagg rvagg force-pushed the rvagg/testing-containers branch 4 times, most recently from b8859e1 to fcc93ea Compare October 27, 2019 08:03
.ci.yml Show resolved Hide resolved
@gengjiawen
Copy link
Member

Also any thought on add ./configure --debug -C for general check (no need to build).

@rvagg
Copy link
Member Author

rvagg commented Nov 1, 2019

Also any thought on add ./configure --debug -C for general check (no need to build).

That'd be fine, these quick checks, like the linter, are ideal to add in here since they are cheap. You just need to have a validation to make sure it's acutally worked. Returning a non-zero exit code is one thing, but has it done what it's supposed to? Maybe you could work up a proposal for how to run and check that?

btw, putting CMake into the standard Ubuntu 18.04 container, like Ninja is, would make it possible to run full CMake builds with this too, which might be a step in a positive direction.

label: Linux Ninja
image: ubuntu1804
execute: |
export CC=/usr/lib/ccache/cc
Copy link
Member

Choose a reason for hiding this comment

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

Is it okay to write those two as env variable in docker image ?

@cclauss
Copy link
Contributor

cclauss commented Nov 4, 2019

Impressive!

@gengjiawen
Copy link
Member

@rvagg Is there any left work to do ? Can we mark this as ready ?

@richardlau
Copy link
Member

@rvagg Is there any left work to do ? Can we mark this as ready ?

@gengjiawen It's not ready, see nodejs/build#1982 (comment)

make lint-py-build PYTHON=python3
make lint-py PYTHON=python3
make lint-py-build PYTHON=python2
make lint-ci PYTHON=python2 || {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't PYTHON be defined before running make instead of after?

Copy link
Member

Choose a reason for hiding this comment

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

You can do either.

PYTHON=python2 make lint-ci

Sets the PYTHON environment variable in the shell and runs make, which turns environment variables into make variables: https://www.gnu.org/software/make/manual/html_node/Environment.html#Environment

make lint-ci PYTHON=python2

Overrides the make variable from the command line: https://www.gnu.org/software/make/manual/html_node/Overriding.html

@BridgeAR
Copy link
Member

BridgeAR commented Jan 2, 2020

Ping @rvagg

1 similar comment
@BridgeAR
Copy link
Member

Ping @rvagg

@BridgeAR BridgeAR force-pushed the master branch 2 times, most recently from 8ae28ff to 2935f72 Compare May 31, 2020 12:19
@jasnell jasnell added the stalled Issues and PRs that are stalled. label Jun 25, 2020
@jasnell
Copy link
Member

jasnell commented Jun 25, 2020

Ping @rvagg (again :-) ...)

@rvagg
Copy link
Member Author

rvagg commented Jul 3, 2020

I've tried and tried to make this work but there's a weird interaction between make and docker that makes it lock up in the middle of a build in these containers with enough frequency for it to be a problem and make this impractical.

@rvagg rvagg closed this Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants