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: fix GYP settings to avoid rebuilding already built files #37429

Closed
wants to merge 3 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Feb 18, 2021

This is my naive attempt at reducing the time it takes to GitHub Actions to run the test suite. My understanding is that a change in #36139 broke ability for the build chain to know which build artefacts are up to date. It attempts to rebuild everything, which may go unnoticed when ccache is setup, but we don't have it setup on GH Actions.

//cc @nodejs/v8

@aduh95 aduh95 requested a review from targos February 18, 2021 11:45
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Feb 18, 2021
@aduh95 aduh95 added the v8 engine Issues and PRs related to the V8 dependency. label Feb 18, 2021
tools/v8_gypfiles/v8.gyp Outdated Show resolved Hide resolved
@aduh95
Copy link
Contributor Author

aduh95 commented Feb 18, 2021

Hum so the build is failing on GH Actions, while the build works on my machine 🥲

@richardlau
Copy link
Member

Hum so the build is failing on GH Actions, while the build works on my machine 🥲

Are you cleaning all build artifacts when you build locally? Actions runs on a clean clone/checkout.

@targos
Copy link
Member

targos commented Feb 19, 2021

At this point I think we need someone who knows makefiles to look at the one generated for run_torque and understand what is the cause of the rebuilds.
I work with ninja and I don't think it is affected.

targos added a commit to targos/node that referenced this pull request Feb 24, 2021
Until we have a fix for the makefiles generated by GYP.

Refs: nodejs#37429
@aduh95
Copy link
Contributor Author

aduh95 commented Feb 24, 2021

This is fixed by #37505.

@aduh95 aduh95 closed this Feb 24, 2021
@aduh95 aduh95 deleted the tests-take-forever branch February 24, 2021 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants