Bug 143219 – Allow building on Windows without Cygwin

• Component: New Bugs
• Status: RESOLVED FIXED

[Myles C. Maxfield]

Allow building on Windows without Cygwin

[Myles C. Maxfield]

Created attachment 249728 [details]
Patch

[Myles C. Maxfield]

Comment on attachment 249728 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249728&action=review

> Source/WebCore/DerivedSources.make:1196
> +	$(DELETE)InspectorOverlayPage.combined.html

Space

[Brent Fulgham]

Comment on attachment 249728 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249728&action=review

>> Source/WebCore/DerivedSources.make:1196
>> +	$(DELETE)InspectorOverlayPage.combined.html
> 
> Space

Can you replied the patch with this fix so we can confirm EWS is happy?

> Source/WebCore/WebCore.vcxproj/migrate-scripts.pl:-64
> -system('/usr/bin/make', '-f', $MIGRATE_SCRIPTS_MAKEFILE, '-j', $NUMCPUS) and die "Failed to build $MIGRATE_SCRIPTS_MAKEFILE: $!";

Nice change! I never noticed this was a Makefile to copy five files.

> Source/WebCore/bindings/scripts/preprocessor.pm:58
> +                push(@args, qw(/EP));

Too much indentation.

> Source/WebCore/bindings/scripts/preprocessor.pm:60
> +                push(@args, qw(-E -P -x c++));

Ditto.

[Brent Fulgham]

Comment on attachment 249728 [details]
Patch

This looks great! Please upload once more with that spacing fix so we can confirm it builds on all platforms. Otherwise, r+.

[Myles C. Maxfield]

Created attachment 249736 [details]
Patch

[Myles C. Maxfield]

Created attachment 249740 [details]
Patch

[Myles C. Maxfield]

Created attachment 249771 [details]
Patch

[Myles C. Maxfield]

Created attachment 249772 [details]
Patch

[Myles C. Maxfield]

Created attachment 249775 [details]
Patch

[Myles C. Maxfield]

Created attachment 249777 [details]
Patch

[Brent Fulgham]

Comment on attachment 249777 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249777&action=review

Looks good. Once EWS is happy go ahead and land it! Thanks!

> Source/WebCore/DerivedSources.make:689
> +    DELETE = cmd //C del

Why two slashes here? I would expect "/C" or maybe "\/C", but not "//C"

> Source/WebCore/WebCore.vcxproj/WebCoreGenerated.make:4
> +    @echo XXWebCoreGeneratedXX > "%ConfigurationBuildDir%\buildfailed"

Did an EOL change get made here? These seem identical to the original state. Maybe double-check EOL is correct before checking in.

[Myles C. Maxfield]

Comment on attachment 249777 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249777&action=review

Overall comments

>> Source/WebCore/DerivedSources.make:689
>> +    DELETE = cmd //C del
> 
> Why two slashes here? I would expect "/C" or maybe "\/C", but not "//C"

http://stackoverflow.com/questions/2463037/calling-windows-commands-e-g-del-from-a-gnu-makefile

[Myles C. Maxfield]

View in context: https://bugs.webkit.org/attachment.cgi?id=249777&action=review

Overall comments

>> Source/WebCore/DerivedSources.make:689
>> +    DELETE = cmd //C del
> 
> Why two slashes here? I would expect "/C" or maybe "\/C", but not "//C"

http://stackoverflow.com/questions/2463037/calling-windows-commands-e-g-del-from-a-gnu-makefile

[Myles C. Maxfield]

Comment on attachment 249777 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=249777&action=review

>> Source/WebCore/WebCore.vcxproj/WebCoreGenerated.make:4
>> +    @echo XXWebCoreGeneratedXX > "%ConfigurationBuildDir%\buildfailed"
> 
> Did an EOL change get made here? These seem identical to the original state. Maybe double-check EOL is correct before checking in.

This is migrating from Windows line endings to Unix line endings, in accordance with our style.

[Brent Fulgham]

(In reply to comment #13)
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249777&action=review
> 
> Overall comments
> 
> >> Source/WebCore/DerivedSources.make:689
> >> +    DELETE = cmd //C del
> > 
> > Why two slashes here? I would expect "/C" or maybe "\/C", but not "//C"
> 
> http://stackoverflow.com/questions/2463037/calling-windows-commands-e-g-del-
> from-a-gnu-makefile

Did you see the last comment in that thread? Maybe GNU make recognizes "//" as an escaped "/" flag, but "\/" is probably clearer to people familiar with escaping syntax in C, shell, etc.

[Myles C. Maxfield]

(In reply to comment #15)
> (In reply to comment #13)
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=249777&action=review
> > 
> > Overall comments
> > 
> > >> Source/WebCore/DerivedSources.make:689
> > >> +    DELETE = cmd //C del
> > > 
> > > Why two slashes here? I would expect "/C" or maybe "\/C", but not "//C"
> > 
> > http://stackoverflow.com/questions/2463037/calling-windows-commands-e-g-del-
> > from-a-gnu-makefile
> 
> Did you see the last comment in that thread? Maybe GNU make recognizes "//"
> as an escaped "/" flag, but "\/" is probably clearer to people familiar with
> escaping syntax in C, shell, etc.

I didn't see that. I'll update after the patch gets landed.

[WebKit Commit Bot]

Comment on attachment 249777 [details]
Patch

Clearing flags on attachment: 249777

Committed r182164: <http://trac.webkit.org/changeset/182164>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.