Skip to content

build,windows: clean lint paths in vcbuild.bat #12278

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 2 commits into from
Closed

build,windows: clean lint paths in vcbuild.bat #12278

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4aab082
build,windows: clean `lint` paths in vcbuild.bat
refack Apr 7, 2017
fefea6b
[personal] Make buildable on vs2017
refack Mar 16, 2017
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
76 changes: 42 additions & 34 deletions vcbuild.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,55 +376,63 @@ goto cpplint
if not defined cpplint goto jslint
echo running cpplint
set cppfilelist=
setlocal enabledelayedexpansion
for /f "tokens=*" %%G in ('dir /b /s /a src\*.c src\*.cc src\*.h ^
test\addons\*.cc test\addons\*.h test\cctest\*.cc test\cctest\*.h ^
test\gc\binding.cc tools\icu\*.cc tools\icu\*.h') do (
set relpath=%%G
set relpath=!relpath:*%~dp0=!
call :add-to-list !relpath!
)
( endlocal
set cppfilelist=%localcppfilelist%
)
set cpp_locs=src\*.c
set cpp_locs=%cpp_locs% src\*.cc
set cpp_locs=%cpp_locs% src\*.h
set cpp_locs=%cpp_locs% test\addons\*.cc
set cpp_locs=%cpp_locs% test\addons\*.h
set cpp_locs=%cpp_locs% test\cctest\*.cc
set cpp_locs=%cpp_locs% test\cctest\*.h
set cpp_locs=%cpp_locs% test\gc\binding.cc
set cpp_locs=%cpp_locs% tools\icu\*.cc
set cpp_locs=%cpp_locs% tools\icu\*.h
for /f "tokens=*" %%G in ('dir /b /s /a:, %cpp_locs%') do call :add-to-list %%G
python tools/cpplint.py %cppfilelist%
python tools/check-imports.py
goto jslint

:add-to-list
echo %1 | findstr /c:"src\node_root_certs.h"
if %errorlevel% equ 0 goto exit

echo %1 | findstr /c:"src\queue.h"
if %errorlevel% equ 0 goto exit

echo %1 | findstr /c:"src\tree.h"
if %errorlevel% equ 0 goto exit

@rem skip subfolders under /src
echo %1 | findstr /r /c:"src\\.*\\.*"
if %errorlevel% equ 0 goto exit

echo %1 | findstr /r /c:"test\\addons\\[0-9].*_.*\.h"
if %errorlevel% equ 0 goto exit

echo %1 | findstr /r /c:"test\\addons\\[0-9].*_.*\.cc"
if %errorlevel% equ 0 goto exit

set "localcppfilelist=%localcppfilelist% %1"
setlocal enabledelayedexpansion
set base_path=
set relpath=%1
set relpath=!relpath:%~dp0=!
endlocal&set relpath=%relpath%
@rem findstr can do multiple searches, but only for regexes
set find_arg=/r
set find_arg=%find_arg% /c:"src\\node_root_certs\.h"
set find_arg=%find_arg% /c:"src\\queue\.h"
set find_arg=%find_arg% /c:"src\\tree\.h"
set find_arg=%find_arg% /c:"src\\.*\\.*"
set find_arg=%find_arg% /c:"test\\addons\\[0-9].*_.*\.h"
set find_arg=%find_arg% /c:"test\\addons\\[0-9].*_.*\.cc"
echo %relpath% | findstr %find_arg% > nul
Copy link
Member

@gibfahn gibfahn Apr 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Does this need a 2> nul as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. findstr just prints matches, which is kinda useless information

if not errorlevel 1 exit /b
set "cppfilelist=%cppfilelist% %relpath%"
exit /b

:find_node
if exist %config%\node set nodeexe=%config%\node&exit /b
where node > nul 2> nul
if not errorlevel 1 set nodeexe=node&exit /b
Copy link
Member

@gibfahn gibfahn Apr 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This picks up the Node in the path if there isn't a local one right? I guess that's alright for linting, as long as we're not doing it for testing as well.

Could you put a comment in to explain that? Something like:

@rem fall back to node.exe in path, used for linting only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll rename the var to nodeexe4linting so noone will use it by accidant

echo Could not file node.exe
Copy link
Member

Choose a reason for hiding this comment

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

Nit: file->find

goto exit
exit /b

:jslint
if defined jslint_ci goto jslint-ci
if not defined jslint goto exit
if not exist tools\eslint\lib\eslint.js goto no-lint
call :find_node
echo running jslint
%config%\node tools\eslint\bin\eslint.js --cache --rule "linebreak-style: 0" --rulesdir=tools\eslint-rules benchmark lib test tools
%nodeexe% tools\eslint\bin\eslint.js --cache --rule "linebreak-style: 0" --rulesdir=tools\eslint-rules benchmark lib test tools
if not errorlevel 1 echo jslint finished with no errors
goto exit

:jslint-ci
call :find_node
echo running jslint-ci
%config%\node tools\jslint.js -J -f tap -o test-eslint.tap benchmark lib test tools
%nodeexe% tools\jslint.js -J -f tap -o test-eslint.tap benchmark lib test tools
if not errorlevel 1 echo jslint-ci finished with no errors
goto exit

:no-lint
Expand All @@ -437,7 +445,7 @@ echo Failed to create vc project files.
goto exit

:help
echo vcbuild.bat [debug/release] [msi] [test-all/test-uv/test-inspector/test-internet/test-pummel/test-simple/test-message] [clean] [noprojgen] [small-icu/full-icu/without-intl] [nobuild] [sign] [x86/x64] [vc2015] [download-all] [enable-vtune] [lint/lint-ci]
echo vcbuild.bat [debug/release] [msi] [test-all/test-uv/test-inspector/test-internet/test-pummel/test-simple/test-message] [clean] [noprojgen] [small-icu/full-icu/without-intl] [nobuild] [sign] [x86/x64] [vc2015] [download-all] [enable-vtune] [lint/lint-ci/jslint/jslint-ci]
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason you're documenting jslint but not cpplint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is no spoon cpplint arg, and the jslint arg was missed in #11856 (comment)

Copy link
Member

@gibfahn gibfahn Apr 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Good point, I didn't notice that there isn't an option to just run cpplint (although there is a cpplint build target). Maybe there should be, there is in the Makefile and it makes sense to have them mirror each other if possible. Doesn't have to be done in this PR though!

echo Examples:
echo vcbuild.bat : builds release build
echo vcbuild.bat debug : builds debug build
Expand Down