Bug 176076 – Do unified source builds for JSC

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Keith Miller]

Created attachment 319296 [details]
Proof of concept (apply to r219089)

We talked about adding a unified source build in the webkit-dev mailing list. This is the bug for the JSC transition.

[Keith Miller]

Note that in the PoC I had to rename Air to not be under B3. I'm not sure that will be needed if we switch to per directory bundles.

[Konstantin Tokarev]

Indeed, do or do not. There is no try.

[Konstantin Tokarev]

BTW, as I probably said before, I propose to use existing WebCore's *AllInOne.cpp infrastructure with fixed AllInOne.cpp files, so that set of files and their order are detereministic

[Keith Miller]

(In reply to Konstantin Tokarev from comment #3)
> BTW, as I probably said before, I propose to use existing WebCore's
> *AllInOne.cpp infrastructure with fixed AllInOne.cpp files, so that set of
> files and their order are detereministic

IIRC, when this was discussed on the webkit-dev mailing list the conclusion was to to do that. The downside of all-in-one cpp files is that we would need to have hundreds of all-in-one cpp files if we want to have ~8 files per bundle. The other alternative is to have a txt file with all the files in the bundle but at that point we might as well have the system do the load balancing for us.

[Keith Miller]

Created attachment 320438 [details]
Patch

[Konstantin Tokarev]

Comment on attachment 320438 [details]
Patch

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

> Source/JavaScriptCore/CMakeLists.txt:1020
> +set(_unifiedSourceCount 0)

I mean, move PROCESS_ALLINONE_FILE from WebCore/WebCoreMacros.cmake to WebKitMacros and use here

[Keith Miller]

Comment on attachment 320438 [details]
Patch

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

>> Source/JavaScriptCore/CMakeLists.txt:1020
>> +set(_unifiedSourceCount 0)
> 
> I mean, move PROCESS_ALLINONE_FILE from WebCore/WebCoreMacros.cmake to WebKitMacros and use here

This is temporary code. I plan to move most of this logic to a separate script before landing so that we can use the same logic between Xcode and CMake.

[Konstantin Tokarev]

Makes sense, parsing C++ files with cmake code never felt right to me

[Keith Miller]

Created attachment 320442 [details]
WIP

[Build Bot]

Attachment 320442 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 62 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 320459 [details]
WIP

[Build Bot]

Attachment 320459 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 320487 [details]
WIP

[Build Bot]

Attachment 320487 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 62 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 320524 [details]
Patch

[Build Bot]

Attachment 320524 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 70 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 320552 [details]
Patch

[Build Bot]

Attachment 320552 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 70 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 320559 [details]
Patch

[Build Bot]

Attachment 320559 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 69 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 320560 [details]
Patch

[Build Bot]

Attachment 320560 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 70 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Joseph Pecoraro]

Comment on attachment 320559 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:8
> +        This patch switchs the CMake JavaScriptCore build to use unified sources.

Typo: switchs => switches

> Source/JavaScriptCore/ChangeLog:269
> +        * CMakeLists.txt:

Double Changelog.

> Source/WTF/ChangeLog:10
> +        important that we use the same script to generate the the bundles

Grammar: "the the" => the

> Source/JavaScriptCore/CMakeLists.txt:1021
> +    if (NOT (${_sourceFile} MATCHES ".*c$"))

How about using just /c$/ the ".*" seems unnecessary here.

> Source/JavaScriptCore/CMakeLists.txt:1034
> +    message("WARNING: unified-source-bundler.rb exited with non-zero status not appending results")

I feel like this should trigger an error / failure. Seems weird that we then fall down to potentially succeed later.

I thin you can do this with:

    message(FATAL_ERROR, "...")

> Source/JavaScriptCore/CMakeLists.txt:1596
> +message(${JavaScriptCore_SOURCES})

This is debug?

[Joseph Pecoraro]

Comment on attachment 320560 [details]
Patch

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

I'd like to see another version of the ruby, since it should be much clearer after addressing comments.

> Source/WTF/generate-unified-source-bundles.rb:1
> +# Copyright (C) 2011, 2016 Apple Inc. All rights reserved.

Copyright date

> Source/WTF/generate-unified-source-bundles.rb:28
> +def usage

This should describe the expected usage, since apparently you use ARGV[0] later on but its not described

     puts "#{File.basename($0)} -p <path> [options] ..."

> Source/WTF/generate-unified-source-bundles.rb:38
> +# Pick an unreasonable number because... whatever.

This comment is unhelpful. Keep the fixme for the unused numbers (or remove them and add them when they are used). But when you do use them provide a descriptive comment explaining them.

You also don't cover these variables in the `usage`. How about just adding them when they are actually used?

> Source/WTF/generate-unified-source-bundles.rb:41
> +$maxCBundleCount = 100000

You said you can't / don't support C files. Drop this one?

> Source/WTF/generate-unified-source-bundles.rb:84
> +    def flush()

Style: Parenthesis not needed here

> Source/WTF/generate-unified-source-bundles.rb:87
> +        if (@currentBundleFile)

Style: Parenthesis are not needed

> Source/WTF/generate-unified-source-bundles.rb:93
> +        @currentBundleFile = $derivedSourcesPath + "UnifiedSource#{($generatedSources.length + 1).to_s + extension}"

I suggest using a local variable for the filename for readability. Also probably using @bundleCount might be clearer than $generatedSources.length:

    nextUnifiedSourceFilename = "UnifiedSource#{@bundleCount}#{extension}"
    @currentBundleFile = $derivedSourcesPath + nextUnifiedSourceFilename

> Source/WTF/generate-unified-source-bundles.rb:100
> +        if (@fileCount % MAX_BUNDLE_SIZE == 0)

Style: Parenthesis are not needed.

Also, can we simplify this to just equality instead of mod arithmetic?

    if @fileCount == MAX_BUNDLE_SIZE

This would require reworking how you call flush. Right now this is somewhat confusing. The first time you call flush it doesn't write anything and instead sets up variables. That is rather confusing. I'd expect `flush` to always write a UnifiedSource and increment if there are contents.

> Source/WTF/generate-unified-source-bundles.rb:110
> +#FIXME: We should figure out how to bundle C files. The lack of namespaces makes this difficult...
> +class CFileManager

I suggest naming this: `NonBundleFileManager` and using it for ".c" and ".m" sources.

How do you handle ".cc" files in WTF which are C++.

> Source/WTF/generate-unified-source-bundles.rb:127
> +$currentDirectory = Pathname.new(ARGV[0]).dirname
> +
> +ARGV.sort.each {

Seems weird to initialize $currentDirectory like this outside the loop. ARGV[0] might be nil.

You should also validate ARGV.size is > 0 and dump usage if it is 0. You expect at least one source file.

> Source/WTF/generate-unified-source-bundles.rb:150
> +$stdout.write($generatedSources.join(";") + ";")

Style: Just use `print ...` instead of `$stdout.write(...)`

[Konstantin Tokarev]

Comment on attachment 320560 [details]
Patch

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

>> Source/WTF/generate-unified-source-bundles.rb:110
>> +class CFileManager
> 
> I suggest naming this: `NonBundleFileManager` and using it for ".c" and ".m" sources.
> 
> How do you handle ".cc" files in WTF which are C++.

These .cc files are 3rd-party code (Google's double-conversion), so I'd argue we don't need to bundle them

[Keith Miller]

Comment on attachment 320560 [details]
Patch

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

>> Source/WTF/generate-unified-source-bundles.rb:1
>> +# Copyright (C) 2011, 2016 Apple Inc. All rights reserved.
> 
> Copyright date

Fixed.

>> Source/WTF/generate-unified-source-bundles.rb:28
>> +def usage
> 
> This should describe the expected usage, since apparently you use ARGV[0] later on but its not described
> 
>      puts "#{File.basename($0)} -p <path> [options] ..."

Updated.

>> Source/WTF/generate-unified-source-bundles.rb:38
>> +# Pick an unreasonable number because... whatever.
> 
> This comment is unhelpful. Keep the fixme for the unused numbers (or remove them and add them when they are used). But when you do use them provide a descriptive comment explaining them.
> 
> You also don't cover these variables in the `usage`. How about just adding them when they are actually used?

I'm going to use them in my next patch so I think it's fine to have a little slop here.

>> Source/WTF/generate-unified-source-bundles.rb:41
>> +$maxCBundleCount = 100000
> 
> You said you can't / don't support C files. Drop this one?

Fixed.

>> Source/WTF/generate-unified-source-bundles.rb:84
>> +    def flush()
> 
> Style: Parenthesis not needed here

fixed.

>> Source/WTF/generate-unified-source-bundles.rb:87
>> +        if (@currentBundleFile)
> 
> Style: Parenthesis are not needed

fixed.

>> Source/WTF/generate-unified-source-bundles.rb:93
>> +        @currentBundleFile = $derivedSourcesPath + "UnifiedSource#{($generatedSources.length + 1).to_s + extension}"
> 
> I suggest using a local variable for the filename for readability. Also probably using @bundleCount might be clearer than $generatedSources.length:
> 
>     nextUnifiedSourceFilename = "UnifiedSource#{@bundleCount}#{extension}"
>     @currentBundleFile = $derivedSourcesPath + nextUnifiedSourceFilename

Done. I actually removed the class member because it's not used outside this function anyway. and I simplified this code.

>> Source/WTF/generate-unified-source-bundles.rb:100
>> +        if (@fileCount % MAX_BUNDLE_SIZE == 0)
> 
> Style: Parenthesis are not needed.
> 
> Also, can we simplify this to just equality instead of mod arithmetic?
> 
>     if @fileCount == MAX_BUNDLE_SIZE
> 
> This would require reworking how you call flush. Right now this is somewhat confusing. The first time you call flush it doesn't write anything and instead sets up variables. That is rather confusing. I'd expect `flush` to always write a UnifiedSource and increment if there are contents.

Done.

>>> Source/WTF/generate-unified-source-bundles.rb:110
>>> +class CFileManager
>> 
>> I suggest naming this: `NonBundleFileManager` and using it for ".c" and ".m" sources.
>> 
>> How do you handle ".cc" files in WTF which are C++.
> 
> These .cc files are 3rd-party code (Google's double-conversion), so I'd argue we don't need to bundle them

We could also just rename them to .cpp files if we want to.

>> Source/WTF/generate-unified-source-bundles.rb:127
>> +ARGV.sort.each {
> 
> Seems weird to initialize $currentDirectory like this outside the loop. ARGV[0] might be nil.
> 
> You should also validate ARGV.size is > 0 and dump usage if it is 0. You expect at least one source file.

Done.

>> Source/WTF/generate-unified-source-bundles.rb:150
>> +$stdout.write($generatedSources.join(";") + ";")
> 
> Style: Just use `print ...` instead of `$stdout.write(...)`

Fixed.

[Keith Miller]

Created attachment 320569 [details]
Patch

[Build Bot]

Attachment 320569 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 70 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Joseph Pecoraro]

Comment on attachment 320569 [details]
Patch

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

The ruby looks much better and more readable to me now.

> Source/WTF/generate-unified-source-bundles.rb:49
> +               ['--max-c-bundle-count', GetoptLong::REQUIRED_ARGUMENT],

Remove

> Source/WTF/generate-unified-source-bundles.rb:62
> +    when '--max-c-bundle-count'
> +        $maxCBundleCount = arg

Remove

> Source/WTF/generate-unified-source-bundles.rb:68
> +usage if !$derivedSourcesPath || ARGV.length == 0

You could do: `ARGV.empty?` which is more natural Ruby than `ARGV.length == 0`

> Source/WTF/generate-unified-source-bundles.rb:114
> +#FIXME: We should figure out how to bundle C files. The lack of namespaces makes this difficult...
> +class CFileManager

A better comment would be: "For files we don't know how to bundle, just pass them through".
This doesn't need to be specific for ".c" files since it may apply to ".m" files as well when you get there.

However, I think you can delete this code altogether! You could avoid creating bundle managers for ".c" and ".m" and instead...

> Source/WTF/generate-unified-source-bundles.rb:143
> +    raise "unknown source extension: #{path.extname} known extensions: #{$bundleManagers.keys.to_s}" unless bundle

... handle them here with:

    bundle = $bundleManagers[path.extname]
    if !bundle
        log("No bundle manger for #{path.extname}: #{path}")
        $generatedSources << file
    else
        bundle.addFile(path)

> Source/WTF/generate-unified-source-bundles.rb:150
> +# Add trailing semicolon since CMake seems dislikes not having it.

Grammar: "since CMake seems dislikes"

> Source/WTF/generate-unified-source-bundles.rb:151
> +# Also, make sure we use print instead of puts because CMake will think the \n is a source file and fail to build.

Don't say "use print instead of puts because ...", that is misleadingly specific. Instead just say "don't include a trailing newline because ...".

> Source/WTF/generate-unified-source-bundles.rb:152
> +print($generatedSources.join(";") + ";")

Style: No need for the parenthesis here:

    print $generatedSources.join(",") + ";"

[Joseph Pecoraro]

(In reply to Build Bot from comment #28)
> Attachment 320569 [details] did not pass style-queue:
> 
> 
> ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control
> clauses should use braces.  [whitespace/braces] [4]
> Total errors found: 1 in 70 files

I wonder if this happens because the line of whitespace (line 219). I wonder if you remove that (or lessen it) if it gets rid of this errant style warning.

[Alex Christensen]

Comment on attachment 320569 [details]
Patch

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

I think a good way to land this would be to land all the parts that make changes that make no effect, then land the rest.

> Source/JavaScriptCore/CMakeLists.txt:49
> -set(JavaScriptCore_SOURCES
> +set(JavaScriptCore_OG_SOURCES

What does OG mean?  I don't think we should use such an abbreviation.

> Source/JavaScriptCore/CMakeLists.txt:1021
> +    if (NOT (${_sourceFile} MATCHES "c$"))

Do we want to check if it ends with .c or just if it ends with c?

> Source/JavaScriptCore/CMakeLists.txt:1039
> +# These are special files that we can't or don't want to unified source compile

Why not?  Should we fix this?

[Keith Miller]

Comment on attachment 320569 [details]
Patch

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

>> Source/JavaScriptCore/CMakeLists.txt:1021
>> +    if (NOT (${_sourceFile} MATCHES "c$"))
> 
> Do we want to check if it ends with .c or just if it ends with c?

I don't think it matters one way or another. I can make it .c if you'd prefer.

>> Source/JavaScriptCore/CMakeLists.txt:1039
>> +# These are special files that we can't or don't want to unified source compile
> 
> Why not?  Should we fix this?

Because it needs special compiler flags non-optimizing compiler flags that we don't want to disable elsewhere.

>> Source/WTF/generate-unified-source-bundles.rb:49
>> +               ['--max-c-bundle-count', GetoptLong::REQUIRED_ARGUMENT],
> 
> Remove

Whoops, Gone.

>> Source/WTF/generate-unified-source-bundles.rb:62
>> +        $maxCBundleCount = arg
> 
> Remove

Gone.

>> Source/WTF/generate-unified-source-bundles.rb:68
>> +usage if !$derivedSourcesPath || ARGV.length == 0
> 
> You could do: `ARGV.empty?` which is more natural Ruby than `ARGV.length == 0`

Oh, I didn't know that existed.

>> Source/WTF/generate-unified-source-bundles.rb:114
>> +class CFileManager
> 
> A better comment would be: "For files we don't know how to bundle, just pass them through".
> This doesn't need to be specific for ".c" files since it may apply to ".m" files as well when you get there.
> 
> However, I think you can delete this code altogether! You could avoid creating bundle managers for ".c" and ".m" and instead...

Yeah, that seems like a better way to do it.

>> Source/WTF/generate-unified-source-bundles.rb:152
>> +print($generatedSources.join(";") + ";")
> 
> Style: No need for the parenthesis here:
> 
>     print $generatedSources.join(",") + ";"

I kinda like calling it as a normal function.

[Keith Miller]

(In reply to Alex Christensen from comment #31)
> Comment on attachment 320569 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=320569&action=review
> 
> I think a good way to land this would be to land all the parts that make
> changes that make no effect, then land the rest.
> 

I don't agree with this. I think it's better to just make sure all the changes make sense and land them. There's no point in trying to wait. That just makes it more likely that we'll have to keep updating the build to ensure it doesn't break between versions of the bundler.

[Keith Miller]

Created attachment 320577 [details]
Patch

[Build Bot]

Attachment 320577 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/jit/BinarySwitch.cpp:219:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 70 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 320577 [details]
Patch

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

r=me

> Source/WTF/ChangeLog:21
> +        up to 8 files from the same directory. We don't bundle files from

Is "from the same directory" still true?

[Keith Miller]

Comment on attachment 320577 [details]
Patch

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

>> Source/WTF/ChangeLog:21
>> +        up to 8 files from the same directory. We don't bundle files from
> 
> Is "from the same directory" still true?

Yeah, I kept it. I tried adding a few files and at was pretty annoying when everything shifts so you have to do a clean build.

[WebKit Commit Bot]

Comment on attachment 320577 [details]
Patch

Clearing flags on attachment: 320577

Committed r221954: <http://trac.webkit.org/changeset/221954>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 320577 [details]
Patch

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

> Source/JavaScriptCore/CMakeLists.txt:49
> -set(JavaScriptCore_SOURCES
> +set(JavaScriptCore_OG_SOURCES

I agree with Alex that the "OG" is cryptic and confusing at best. We should rename this to something meaningful and descriptive.

Perhaps `JavaScriptCore_SOURCE_IN` to be in line with our `foo.in` files which is an indication it needs to be processed.

JavaScriptCore_UNBUNDLED_SOURCES, JavaScriptCore_ALL_SOURCES, JavaScriptCore_x_SOURCES.

At this point just about anything is better than OG.

[Michael Catanzaro]

Comment on attachment 320577 [details]
Patch

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

> Source/WTF/ChangeLog:32
> +        * generate-unified-source-bundles.rb: Added.

I don't think this script should be under WTF, it should be under Tools/Scripts/webkitruby. Almost all of the scripts that are required by the build are already kept under Tools; no need for this to be an exception.

>> Source/JavaScriptCore/CMakeLists.txt:49
>> +set(JavaScriptCore_OG_SOURCES
> 
> I agree with Alex that the "OG" is cryptic and confusing at best. We should rename this to something meaningful and descriptive.
> 
> Perhaps `JavaScriptCore_SOURCE_IN` to be in line with our `foo.in` files which is an indication it needs to be processed.
> 
> JavaScriptCore_UNBUNDLED_SOURCES, JavaScriptCore_ALL_SOURCES, JavaScriptCore_x_SOURCES.
> 
> At this point just about anything is better than OG.

I haven't been following this bug closely, but I was going to complain about this too. I also have no clue what OG stands for.

> Source/JavaScriptCore/CMakeLists.txt:1033
> +    message(FATAL_ERROR "unified-source-bundler.rb exited with non-zero status not appending results")

Is this missing a comma? Since the error is fatal, a better message would be simply "unified-source-bundler.rb exited with non-zero status"

[Keith Miller]

(In reply to Michael Catanzaro from comment #41)
> Comment on attachment 320577 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=320577&action=review
> 
> > Source/WTF/ChangeLog:32
> > +        * generate-unified-source-bundles.rb: Added.
> 
> I don't think this script should be under WTF, it should be under
> Tools/Scripts/webkitruby. Almost all of the scripts that are required by the
> build are already kept under Tools; no need for this to be an exception.

Due to Apple's internal build system rules, we need this script to be in one of the Source subdirectories. You'll notice that all the Xcode scripts are duplicated across directories. I'd like to use the same script for all the projects in WebKit that we will do unified sources with. Which, unfortunately, means that the script needs to be forwarded from the WTF.

> 
> >> Source/JavaScriptCore/CMakeLists.txt:49
> >> +set(JavaScriptCore_OG_SOURCES
> > 
> > I agree with Alex that the "OG" is cryptic and confusing at best. We should rename this to something meaningful and descriptive.

> > 
> > Perhaps `JavaScriptCore_SOURCE_IN` to be in line with our `foo.in` files which is an indication it needs to be processed.
> > 
> > JavaScriptCore_UNBUNDLED_SOURCES, JavaScriptCore_ALL_SOURCES, JavaScriptCore_x_SOURCES.
> > 
> > At this point just about anything is better than OG.
> 
> I haven't been following this bug closely, but I was going to complain about
> this too. I also have no clue what OG stands for.

Whoops, I meant to change this but I forgot... :/

> 
> > Source/JavaScriptCore/CMakeLists.txt:1033
> > +    message(FATAL_ERROR "unified-source-bundler.rb exited with non-zero status not appending results")
> 
> Is this missing a comma? Since the error is fatal, a better message would be
> simply "unified-source-bundler.rb exited with non-zero status"

If you mean a comma between "FATAL_ERROR" and "unified-source-bundler.rb...", no because CMake is weird. I agree that the "not appending results" part of the message is unnecessary.

[Keith Miller]

I uploaded a follow-up patch: https://bugs.webkit.org/show_bug.cgi?id=176823

[Radar WebKit Bug Importer]

<rdar://problem/34694175>