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

Add two tips for speeding up the dev builds #36452

Closed
wants to merge 0 commits into from

Conversation

mmomtchev
Copy link
Contributor

Add two tips for speeding up the dev builds

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem. labels Dec 9, 2020
BUILDING.md Outdated Show resolved Hide resolved
BUILDING.md Outdated Show resolved Hide resolved
BUILDING.md Outdated
If you plan to frequently rebuild Node.js, especially if using several branches,
installing `ccache` can help to greatly reduce build times. Setup with:
```console
$ sudo apt install ccache # for Debian/Ubuntu, included in most Linux distros
Copy link
Member

Choose a reason for hiding this comment

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

Probably a good idea to include how to install ccache on max and windows as well in a comment?

brew install ccache
choco install ccache 

And maybe link to https://ccache.dev/download.html ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OSX is flawless, but support for Visual Studio is far from perfect, I don't know if we should recommend to people to use it?

BUILDING.md Outdated
$ ccache -o cache_dir=<tmp_dir>
$ ccache -o max_size=5.0G
$ export CC="ccache gcc" # add to your .profile
$ export CXX="ccache g++" # add to your .profile
Copy link
Member

Choose a reason for hiding this comment

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

I would mention that in windows this is set and not export for the windows users?

(To be fair I've never used ccache in windows)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think ccache officially works with VS on Windows. I know in the CI we don't use it (we use clcache instead https://github.com/nodejs/build/blob/36202a24f7d19729bea1e0efb0fe7045b2c42dc6/ansible/roles/visual-studio/tasks/main.yml#L27-L54).

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Good tips, left some nits

BUILDING.md Outdated Show resolved Hide resolved
BUILDING.md Outdated Show resolved Hide resolved
Copy link
Contributor

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

This is great! ❤️

BUILDING.md Outdated Show resolved Hide resolved
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Left a comment about the anchor format but that's easy to fix. I agree that more can be done to show how to make it work in non apt-get environments, but those can be subsequent PRs. I'm OK with "I'm only PR'ing in the platforms I actually use a lot." This is useful as is and I'm totally OK if someone wants to land it.

@Trott Trott closed this Dec 18, 2020
Trott pushed a commit that referenced this pull request Dec 18, 2020
Add two important tips for novice Node.js contributors

PR-URL: #36452
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member

Trott commented Dec 18, 2020

Landed in 36581f1

targos pushed a commit that referenced this pull request Dec 21, 2020
Add two important tips for novice Node.js contributors

PR-URL: #36452
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request May 1, 2021
Add two important tips for novice Node.js contributors

PR-URL: #36452
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
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. doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants