| Summary: | Update filter-build-webkit | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||
| Component: | Tools / Tests | Assignee: | BJ Burg <bburg> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | bburg, darin, simon.fraser, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
BJ Burg
2021-03-19 11:15:24 PDT
Created attachment 423750 [details]
Patch v1.0
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? 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. Committed r274744 (235561@main): <https://commits.webkit.org/235561@main> 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 \.$ |