Bug 202477 – [ews] wincairo queue should use del instead of rm command

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

[Aakash Jain]

Windows doesn't have 'rm' command. So, windows or wincairo queue should use 'del' instead of 'rm' command in the CleanUpGitIndexLock build-step.

[Aakash Jain]

wincairo-ews-002 bot got in an infinite loop when I cancelled a build https://ews-build.webkit.org/#/builders/12/builds/5786 because of .git/index.lock file, and failure to run the 'rm' command.

Subsequent builds on this bot after #5786:
https://ews-build.webkit.org/#/builders/12/builds/5794
https://ews-build.webkit.org/#/builders/12/builds/5795
https://ews-build.webkit.org/#/builders/12/builds/5796

[Aakash Jain]

Created attachment 380032 [details]
Patch

[Jonathan Bedard]

Comment on attachment 380032 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:621
> +        self.setProperty('platform', 'wincairo')

What is our property in test_success by default?

[Aakash Jain]

Comment on attachment 380032 [details]
Patch

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

>> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:621
>> +        self.setProperty('platform', 'wincairo')
> 
> What is our property in test_success by default?

platform is not set in test_success, so it will be on the steps.py code to handle it, which uses '*' as default platform (see line 126 of steps.py in this patch).

[WebKit Commit Bot]

Comment on attachment 380032 [details]
Patch

Clearing flags on attachment: 380032

Committed r250616: <https://trac.webkit.org/changeset/250616>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55917272>

[Dean Johnson]

Comment on attachment 380032 [details]
Patch

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

> Tools/BuildSlaveSupport/ews-build/steps.py:128
> +        if platform in ('win', 'wincairo'):

Nit: I think this can be simplified by passing a tuple to platform.startswith
```py
if platform.startswith(('win', 'wincairo')):
```

[Don Olmstead]

Comment on attachment 380032 [details]
Patch

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

Should the commands be abstracted at all with a windows shell vs a cygwin/nix shell?

>> Tools/BuildSlaveSupport/ews-build/steps.py:128
>> +        if platform in ('win', 'wincairo'):
> 
> Nit: I think this can be simplified by passing a tuple to platform.startswith
> ```py
> if platform.startswith(('win', 'wincairo')):
> ```

Isn't AppleWin using cygwin? Also I'm not sure what Brent has in store for FTW until it potentially becomes the Windows port.

[Aakash Jain]

(In reply to Don Olmstead from comment #9)
> Isn't AppleWin using cygwin? 
You are right. I will create a follow up patch.

[Aakash Jain]

(In reply to Dean Johnson from comment #8)
> Nit: I think this can be simplified by passing a tuple to platform.startswith
> if platform.startswith(('win', 'wincairo')):
Agree. However, as we are not using 'win' anymore, I am going to change it to simply
platform == 'wincairo'

[Aakash Jain]

Reopening to attach new patch.

[Aakash Jain]

Created attachment 380162 [details]
Patch

[Aakash Jain]

Committed r250736: <https://trac.webkit.org/changeset/250736>