| Summary: | Incorrect handling of findstr results in *.vcproj | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Hiroaki Nakamura <hnakamur> | ||||||||||
| Component: | Tools / Tests | Assignee: | Adam Roben (:aroben) <aroben> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Trivial | CC: | aroben | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 523.x (Safari 3) | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows Vista | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Hiroaki Nakamura
2007-11-03 00:27:33 PDT
Created attachment 17014 [details]
Fix handling results of findstr
Comment on attachment 17014 [details]
Fix handling results of findstr
Please set the "review?" flag on patches in the future to make sure they're reviewed. Thanks!
Comment on attachment 17014 [details]
Fix handling results of findstr
Usage of "if ERRORLEVEL" was wrong.
I edit my attachment and changed review flag to "+". Is this what you mean? Thanks. (In reply to comment #4) > I edit my attachment and changed review flag to "+". Is this what you mean? > Thanks. > No, set it to ? so a WebKit reviewer knows to take a look at it. They'll change it to +/-. Oh, now I understand, you had set the review flag which I should have set it. Sorry to bother you and thank you. Comment on attachment 17014 [details]
Fix handling results of findstr
I'm not sure how the new behavior is different from the old. Before we had:
if ERRORLEVEL 0 set EnablePREfast false else set EnablePREfast true
Now we have
if ERRORLEVEL 1 set EnablePREfast true else set EnablePREfast false
What is the difference?
I also don't think we want "echo on" in our pre-build step.
Comment on attachment 17014 [details]
Fix handling results of findstr
Index: WebKit/win/WebKit.vcproj/WebKit.vcproj
===================================================================
--- WebKit/win/WebKit.vcproj/WebKit.vcproj (revision 27388)
+++ WebKit/win/WebKit.vcproj/WebKit.vcproj (working copy)
@@ -25,7 +25,7 @@
>
<Tool
Name="VCPreBuildEventTool"
- CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 0 set EnablePREfast="false" else set EnablePREfast="true"
if ERRORLEVEL 0 set AnalyzeWithLargeStack="" AnalyzeWithLargeStack="/analyze:65536"

mkdir 2>NUL "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"
xcopy /y /d "$(WebKitLibrariesDir)\include\JavaScriptCore\JavaScriptCore\*" "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"

bash auto-version.sh "$(InputDir)." "$(IntDir)"

"
+ CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 1 (set EnablePREfast="true") else set EnablePREfast="false"
if ERRORLEVEL 1 (set AnalyzeWithLargeStack="/analyze:65536") else set AnalyzeWithLargeStack=""

mkdir 2>NUL "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"
xcopy /y /d "$(WebKitLibrariesDir)\include\JavaScriptCore\JavaScriptCore\*" "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"

bash auto-version.sh "$(InputDir)." "$(IntDir)"

"
/>
<Tool
Name="VCCustomBuildTool"
@@ -119,7 +119,7 @@
>
<Tool
Name="VCPreBuildEventTool"
- CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 0 set EnablePREfast="false" else set EnablePREfast="true"
if ERRORLEVEL 0 set AnalyzeWithLargeStack="" AnalyzeWithLargeStack="/analyze:65536"

mkdir 2>NUL "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"
xcopy /y /d "$(WebKitLibrariesDir)\include\JavaScriptCore\JavaScriptCore\*" "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"

bash auto-version.sh "$(InputDir)." "$(IntDir)"

"
+ CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 1 (set EnablePREfast="true") else set EnablePREfast="false"
if ERRORLEVEL 1 (set AnalyzeWithLargeStack="/analyze:65536") else set AnalyzeWithLargeStack=""

mkdir 2>NUL "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"
xcopy /y /d "$(WebKitLibrariesDir)\include\JavaScriptCore\JavaScriptCore\*" "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"

bash auto-version.sh "$(InputDir)." "$(IntDir)"

"
/>
<Tool
Name="VCCustomBuildTool"
@@ -211,7 +211,7 @@
>
<Tool
Name="VCPreBuildEventTool"
- CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 0 set EnablePREfast="false" else set EnablePREfast="true"
if ERRORLEVEL 0 set AnalyzeWithLargeStack="" AnalyzeWithLargeStack="/analyze:65536"

mkdir 2>NUL "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"
xcopy /y /d "$(WebKitLibrariesDir)\include\JavaScriptCore\JavaScriptCore\*" "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"

bash auto-version.sh "$(InputDir)." "$(IntDir)"

"
+ CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 1 (set EnablePREfast="true") else set EnablePREfast="false"
if ERRORLEVEL 1 (set AnalyzeWithLargeStack="/analyze:65536") else set AnalyzeWithLargeStack=""

mkdir 2>NUL "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"
xcopy /y /d "$(WebKitLibrariesDir)\include\JavaScriptCore\JavaScriptCore\*" "$(WebKitOutputDir)\include\JavaScriptCore\JavaScriptCore"

bash auto-version.sh "$(InputDir)." "$(IntDir)"

"
/>
<Tool
Name="VCCustomBuildTool"
Index: WebCore/WebCore.vcproj/WebCore.vcproj
===================================================================
--- WebCore/WebCore.vcproj/WebCore.vcproj (revision 27388)
+++ WebCore/WebCore.vcproj/WebCore.vcproj (working copy)
@@ -26,7 +26,7 @@
>
<Tool
Name="VCPreBuildEventTool"
- CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 0 set EnablePREfast="false" else set EnablePREfast="true"
if ERRORLEVEL 0 set AnalyzeWithLargeStack="" AnalyzeWithLargeStack="/analyze:65536"
bash build-generated-files.sh "$(WebKitOutputDir)" "$(WebKitLibrariesDir)"
"
+ CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 1 (set EnablePREfast="true") else set EnablePREfast="false"
if ERRORLEVEL 1 (set AnalyzeWithLargeStack="/analyze:65536") else set AnalyzeWithLargeStack=""
bash build-generated-files.sh "$(WebKitOutputDir)" "$(WebKitLibrariesDir)"
"
/>
<Tool
Name="VCCustomBuildTool"
@@ -101,7 +101,7 @@
>
<Tool
Name="VCPreBuildEventTool"
- CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 0 set EnablePREfast="false" else set EnablePREfast="true"
if ERRORLEVEL 0 set AnalyzeWithLargeStack="" AnalyzeWithLargeStack="/analyze:65536"
bash build-generated-files.sh "$(WebKitOutputDir)" "$(WebKitLibrariesDir)"
"
+ CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 1 (set EnablePREfast="true") else set EnablePREfast="false"
if ERRORLEVEL 1 (set AnalyzeWithLargeStack="/analyze:65536") else set AnalyzeWithLargeStack=""
bash build-generated-files.sh "$(WebKitOutputDir)" "$(WebKitLibrariesDir)"
"
/>
<Tool
Name="VCCustomBuildTool"
@@ -172,7 +172,7 @@
>
<Tool
Name="VCPreBuildEventTool"
- CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 0 set EnablePREfast="false" else set EnablePREfast="true"
if ERRORLEVEL 0 set AnalyzeWithLargeStack="" AnalyzeWithLargeStack="/analyze:65536"
bash build-generated-files.sh "$(WebKitOutputDir)" "$(WebKitLibrariesDir)"
"
+ CommandLine="set PATH=%SystemDrive%\cygwin\bin;%PATH%
touch "$(WebKitOutputDir)\tmp.cpp"
cl /analyze /nologo /c "$(WebKitOutputDir)\tmp.cpp" 2>&1 | findstr D9040
if ERRORLEVEL 1 (set EnablePREfast="true") else set EnablePREfast="false"
if ERRORLEVEL 1 (set AnalyzeWithLargeStack="/analyze:65536") else set AnalyzeWithLargeStack=""
bash build-generated-files.sh "$(WebKitOutputDir)" "$(WebKitLibrariesDir)"
"
/>
<Tool
Name="VCCustomBuildTool"
Created attachment 17176 [details]
Fix handling results of findstr
First, "echo on" was my mistake. I forgot removing them.
As for ERRORLEVEL, I read the help of cmd.exe and understand that "if ERRORLEVEL n stmt1 else stmt2" means if errorlevel is greater than or equal to n then execute stmt1 else execute stmt2. And findstr returns 0 when found, 1 when not found. So if you write "if ERRORLEVEL 0", stmt1 is always executed.
Also you need parenthesis if stmt1 is not one word. I tried without parenthesis and it didn't work correctly. I tried on Japanese Windows Vista Home Edition.
And original project file was missing "else" for setting AnalyzeWithLargeStack.
Comment on attachment 17176 [details]
Fix handling results of findstr
Thanks for the explanation.
It seems to me we could combine the two if/else statements into one. But r=me even if you don't do that. Thanks!
This doesn't apply any more :( Created attachment 17208 [details]
patch to revision 27716
Hi, Mark. I recreated the patch for revision 27716. Please try again.
Comment on attachment 17208 [details]
patch to revision 27716
r=me again
Adam, could you land this patch, or clear the review flag if the patch is one again impossible to apply. I'd like this out of our "reviewed by not committed" list. Created attachment 27261 [details] Revised to apply against ToT (@r40493) Comment on attachment 27261 [details] Revised to apply against ToT (@r40493) r=me Committed in revision 40520. |