Bug 230104 – [build.webkit.org] Only display filtered build logs

• Status: ASSIGNED
• Component: Tools / Tests

[Aakash Jain]

Only display filtered build logs for build.webkit.org using filter-build-webkit script. This would also help in keeping the buildbot instance speedy.

[Aakash Jain]

Similar to rdar://82628702

[Aakash Jain]

Created attachment 437752 [details]
Patch

[Aakash Jain]

Sample run: https://build.webkit-dev.org/#/builders/49/builds/5

[Darin Adler]

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.

[Aakash Jain]

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.

[Aakash Jain]

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.

[Darin Adler]

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.

[Aakash Jain]

(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.

[Alexey Proskuryakov]

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.

[Darin Adler]

(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.

[Aakash Jain]

Removing the old "./" in Bug 230165 to make build.webkit.org configuration consistent with EWS.

[Radar WebKit Bug Importer]

<rdar://problem/91681929>