Bug 223528 – Update filter-build-webkit

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Blaze Burg]

.

[Blaze Burg]

Created attachment 423750 [details]
Patch v1.0

[Simon Fraser (smfr)]

Comment on attachment 423750 [details]
Patch v1.0

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

> Tools/Scripts/filter-build-webkit:352
> +    return 1 if $line =~ /^ld: warning: linking with (.+) but not using any symbols from it/;
> +    return 1 if $line =~ /^ld: warning: dylib \(.*\) was built for newer .* version (.*) than being linked (.*)$/;

Do we really want to filter these?

[Blaze Burg]

Comment on attachment 423750 [details]
Patch v1.0

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

>> Tools/Scripts/filter-build-webkit:352
>> +    return 1 if $line =~ /^ld: warning: dylib \(.*\) was built for newer .* version (.*) than being linked (.*)$/;
> 
> Do we really want to filter these?

I'll remove the ld: warning: lines, as they may be actionable.

[Blaze Burg]

Committed r274744 (235561@main): <https://commits.webkit.org/235561@main>

[Radar WebKit Bug Importer]

<rdar://problem/75639935>

[Darin Adler]

Comment on attachment 423750 [details]
Patch v1.0

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

A few comments nitpicking some of our regular expressions

> Tools/Scripts/filter-build-webkit:360
>      return 1 if $line =~ /.* com.apple.actool.compilation-results .*/;

Strange that we used .* at the beginning and end; not needed and goes without saying.

> Tools/Scripts/filter-build-webkit:362
>      return 1 if $line =~ /.*\/Assets.car/;
>      return 1 if $line =~ /.*\/assetcatalog_generated_info.plist/;

Strange that we used .* at the beginning; not needed and goes without saying.

> Tools/Scripts/filter-build-webkit:366
> +    return 1 if $line =~ /^Unlocking '.*\.keychain-db'$/;

probably want this to be .+\.

> Tools/Scripts/filter-build-webkit:369
> +    return 1 if $line =~ /^The list of exported symbols did not change.$/;

That period is a single character match, not a period. Need \. for a period.

> Tools/Scripts/filter-build-webkit:372
> +    return 1 if $line =~ /^Adding .*?entitlements/;

This should just be .* there's no need to add the ?. The ? makes it non-greedy, which has no effect in this case. It’s already optional, because * means 0 or more matches.

> Tools/Scripts/filter-build-webkit:375
> +    return 1 if $line =~ /^.* will not be code signed because its settings don't specify a development team.$/;

Start should probably be .+, and should probably be \.$