Skip to content

[vcbuild.bat] bug fix & add ccache support #52126

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

[vcbuild.bat] bug fix & add ccache support #52126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a43a5eb
[vcbuild.bat] minor fix
congzhangzh Mar 17, 2024
c6b09b1
[vcbuild.bat] add ccache support
congzhangzh Mar 17, 2024
e41f6f7
[vcbuild.bat] minor fix
congzhangzh Mar 17, 2024
650eb56
[vcbuild.bat][ccache] fix
congzhangzh Mar 17, 2024
66a6e5e
[vcbuild.bat] disable vcpkg, as it will cause the zlib1 problem
congzhangzh Mar 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions props_4_ccache.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project>
<ItemDefinitionGroup>
<ClCompile>
<!-- /Z7 of cl.exe, ref: https://learn.microsoft.com/en-us/cpp/build/reference/z7-zi-zi-debug-information-format -->
<DebugInformationFormat>OldStyle</DebugInformationFormat>
<ObjectFileName>$(IntDir)%(FileName).obj</ObjectFileName>
</ClCompile>
</ItemDefinitionGroup>
</Project>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add an empty line at the bottom?

Also, I think a better place for this file would be in tools/msvs as there are all Windows-specific tools. Once you move it, just make sure to change the path in ForceImportAfterCppProps in vcbuild.bat too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, got it :)

11 changes: 8 additions & 3 deletions vcbuild.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ set v8_build_options=
set http2_debug=
set nghttp2_debug=
set link_module=
set ccache_path=
set no_cctest=
set cctest=
set openssl_no_asm=
Expand Down Expand Up @@ -138,6 +139,7 @@ if /i "%1"=="static" set enable_static=1&goto arg-ok
if /i "%1"=="no-NODE-OPTIONS" set no_NODE_OPTIONS=1&goto arg-ok
if /i "%1"=="debug-nghttp2" set debug_nghttp2=1&goto arg-ok
if /i "%1"=="link-module" set "link_module= --link-module=%2%link_module%"&goto arg-ok-2
if /i "%1"=="ccache" set "ccache_path=%2%"&goto arg-ok-2
if /i "%1"=="no-cctest" set no_cctest=1&goto arg-ok
if /i "%1"=="cctest" set cctest=1&goto arg-ok
if /i "%1"=="openssl-no-asm" set openssl_no_asm=1&goto arg-ok
Expand Down Expand Up @@ -325,10 +327,12 @@ if "%target%"=="Build" (
)
if "%target%"=="node" if exist "%config%\cctest.exe" del "%config%\cctest.exe"
if defined msbuild_args set "extra_msbuild_args=%extra_msbuild_args% %msbuild_args%"
if defined ccache_path set "extra_msbuild_args=%extra_msbuild_args% /p:TrackFileAccess=false /p:CLToolPath=%ccache_path% /p:ForceImportAfterCppProps=%CD%\props_4_ccache.props"
@rem Setup env variables to use multiprocessor build
set UseMultiToolTask=True
set EnforceProcessCountAcrossBuilds=True
set MultiProcMaxCount=%NUMBER_OF_PROCESSORS%
set "extra_msbuild_args=%extra_msbuild_args% /p:UseMultiToolTask=True"
set "extra_msbuild_args=%extra_msbuild_args% /p:EnforceProcessCountAcrossBuilds=True"
set "extra_msbuild_args=%extra_msbuild_args% /p:MultiProcMaxCount=%NUMBER_OF_PROCESSORS%"
set "extra_msbuild_args=%extra_msbuild_args% /p:VcpkgEnabled=false"
Copy link
Contributor

Choose a reason for hiding this comment

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

This line was not here before (the other 3 are rewritten). Is this connected with your other PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Partially, if this pull request is approved, the https://github.com/nodejs/node/pull/52181/files will be upgraded to use vcbuild ccache switch directly:)

I'm a little worried, this pr changed too much, which is hard to accept, so I decided to split it out or drop this pull request, but use cmdline to config msbbuild directly, which is enough to make it work.

Btw, I don't like vcbuild.bat, why not use configure.py directly, The windows support for developing node itself is not so good, maybe most guys use linux/mac:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, maybe that's my fault, it seems just setting environment variable is enough, I will check it later

https://learn.microsoft.com/en-us/visualstudio/msbuild/how-to-use-environment-variables-in-a-build?view=vs-2022

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@StefanStojanovic Hi Stefan, sorry about my fault, it works by setting msbuild property directly

I do a test

set TrackFileAccess=false
set UseMultiToolTask=true
: allow link symbols redefine
set ForceImportAfterCppTargets=some_build.targets
msbuild ConsoleApplication_with_symbol_redefine.sln

and it works!

msbuild node.sln %msbcpu% /t:%target% /p:Configuration=%config% /p:Platform=%msbplatform% /clp:NoItemAndPropertyList;Verbosity=minimal /nologo %extra_msbuild_args%
if errorlevel 1 (
if not defined project_generated echo Building Node with reused solution failed. To regenerate project files use "vcbuild projgen"
Expand Down Expand Up @@ -716,6 +720,7 @@ echo vcbuild.bat test : builds debug build and runs tests
echo vcbuild.bat build-release : builds the release distribution as used by nodejs.org
echo vcbuild.bat enable-vtune : builds Node.js with Intel VTune profiling support to profile JavaScript
echo vcbuild.bat link-module my_module.js : bundles my_module as built-in module
echo vcbuild.bat ccache c:\ccache\ : use ccache to speed build
echo vcbuild.bat lint : runs the C++, documentation and JavaScript linter
echo vcbuild.bat no-cctest : skip building cctest.exe
goto exit
Expand Down