Only display filtered build logs for build.webkit.org using filter-build-webkit script. This would also help in keeping the buildbot instance speedy.
Similar to rdar://82628702
Created attachment 437752 [details] Patch
Sample run: https://build.webkit-dev.org/#/builders/49/builds/5
Comment on attachment 437752 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437752&action=review > Tools/CISupport/build-webkit-org/steps_unittest.py:423 > + command='perl ./Tools/Scripts/build-webkit --release --prefix=/app/webkit/WebKitBuild/Release/install --gtk | Tools/Scripts/filter-build-webkit', Why are we using a "./" prefix for build-webkit but not for filter-build-webkit? I think it should be used in both cases, since we don’t want $PATH involved.
Comment on attachment 437752 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437752&action=review >> Tools/CISupport/build-webkit-org/steps_unittest.py:423 >> + command='perl ./Tools/Scripts/build-webkit --release --prefix=/app/webkit/WebKitBuild/Release/install --gtk | Tools/Scripts/filter-build-webkit', > > Why are we using a "./" prefix for build-webkit but not for filter-build-webkit? I think it should be used in both cases, since we don’t want $PATH involved. No specific reason apart from "./" being in this configuration from past. Most of our recent buildbot configuration doesn't have "./". This should work without $PATH being involved, as it's still relative path from current directory.
Alexey and Ryan don't think it's a good idea since we might need full logs in certain circumstances. Will skip it for now, until we have another way to get full logs when required.
Comment on attachment 437752 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437752&action=review >>> Tools/CISupport/build-webkit-org/steps_unittest.py:423 >>> + command='perl ./Tools/Scripts/build-webkit --release --prefix=/app/webkit/WebKitBuild/Release/install --gtk | Tools/Scripts/filter-build-webkit', >> >> Why are we using a "./" prefix for build-webkit but not for filter-build-webkit? I think it should be used in both cases, since we don’t want $PATH involved. > > No specific reason apart from "./" being in this configuration from past. Most of our recent buildbot configuration doesn't have "./". This should work without $PATH being involved, as it's still relative path from current directory. I think you misunderstood my point. I was talking about relying on $PATH to find the script, bur rather the inverse. Typically the reason one would specify "./" is to make sure that a similarly named script in $PATH does not get used by accident, to avoid unwanted $PATH. But of course the chances that there is another "Tools/Scripts/filter-build-webkit" are so small it’s not a practical concern. I know you’re not planning on landing the patch, so not important. But generally we want to be consistent.
(In reply to Darin Adler from comment #7) > I think you misunderstood my point. I was talking about relying on $PATH to find the script, bur rather the inverse. Typically the reason one would specify "./" is to make sure that a similarly named script in $PATH does not get used by accident, to avoid unwanted $PATH. But of course the chances that there is another "Tools/Scripts/filter-build-webkit" are so small it’s not a practical concern. ok, got it. Agree that the chances of having another "Tools/Scripts/filter-build-webkit" in $PATH are very small.
Is PATH actually involved when shell looks for the tool? E.g. this fails even though jsc-stress-test-helpers is a directory in Tools/Scripts, and that is in my PATH. $ jsc-stress-test-helpers/bytecode-cache-test-helper.sh -bash: jsc-stress-test-helpers/bytecode-cache-test-helper.sh: No such file or directory So I think that "./" is only relevant when there isn't any path specified in the invocation. Agreed that we should target a consistent style.
(In reply to Alexey Proskuryakov from comment #9) > So I think that "./" is only relevant when there isn't any path specified in > the invocation. Oh, OK my mistake. Didn't realize the path wasn’t involved in that case. I think the old use of "./" here is mysterious, then. Seems like perl probably doesn’t need that "./". Not important, just peculiar.
Removing the old "./" in Bug 230165 to make build.webkit.org configuration consistent with EWS.
<rdar://problem/91681929>