Replace all copies of _dumbwin32proc.py

RESOLVED FIXED

Status

Infrastructure & Operations
CIDuty
P2
normal
RESOLVED FIXED
6 years ago
a month ago

People

(Reporter: coop, Assigned: armenzg)

Tracking

Details

(Whiteboard: [opsi], URL)

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
While doing work on the new WinXP ref platform (bug 770579), I discovered that we're still replacing the copy of _dumbwin32proc.py that lives under C:\mozilla-build\python25\Lib\site-packages\twisted\internet. Another copy of that module als o gets installed under C:\mozilla-build\buildbotve\python25\Lib\site-packages\twisted\internet. I'm uncertain whether this other copy is ever called, but we should probably be replacing both version to be safe.

https://hg.mozilla.org/build/opsi-package-sources/file/e55c081cb8cf/twisted_dumbwin32proc/CLIENT_DATA/twisted_dumbwin32proc.ins
(Assignee)

Comment 1

6 years ago
Probably safe to compare the .bak file with the other one you want to replace:
C:\mozilla-build\python25\Lib\site-packages\twisted\internet\_dumbwin32proc.py.bak
C:\mozilla-build\buildbotve\python25\Lib\site-packages\twisted\internet\_dumbwin32proc.py

Great to see that you caught this!
(Reporter)

Comment 2

6 years ago
Created attachment 688766 [details] [diff] [review]
Replace the version of _dumbwin32proc.py under buildbotve also
Attachment #688766 - Flags: review?(armenzg)
(Assignee)

Updated

6 years ago
Attachment #688766 - Flags: review?(armenzg) → review+
(Reporter)

Comment 3

6 years ago
Comment on attachment 688766 [details] [diff] [review]
Replace the version of _dumbwin32proc.py under buildbotve also

https://hg.mozilla.org/build/opsi-package-sources/rev/ac29b3f0987c
Attachment #688766 - Flags: checked-in+
(Reporter)

Comment 4

6 years ago
I've marked all the XP slaves to re-install this package on next boot. I'll check on them tomorrow to make sure I didn't inadvertently tank the whole pool.
(Reporter)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 5

5 years ago
We need to work on this again:
C:\Documents and Settings\cltbld>dir C:\mozilla-build\buildbotve\python25\Lib\site-packages\twisted\internet\_dumbwin32proc.py*
The system cannot find the path specified.

We got the path wrong:
C:\mozilla-build\buildbotve\Lib\site-packages\twisted\internet
versus
C:\mozilla-build\buildbotve\python25\Lib\site-packages\twisted\internet

This is what I did on a w7 and an xp slave:
cd C:\mozilla-build\buildbotve\Lib\site-packages\twisted\internet
del _dumbwin32proc.pyc
move _dumbwin32proc.py _dumbwin32proc.py.bak
wget -q --no-check-certificate https://hg.mozilla.org/build/opsi-package-sources/raw-file/e55c081cb8cf/twisted_dumbwin32proc/CLIENT_DATA/_dumbwin32proc.py

I assume we need to do the same on win64 and win8 slaves.

#########################################
diff -pU 8 _dumbwin32proc.py _dumbwin32proc.py.bak
--- _dumbwin32proc.py   Wed Mar 13 10:41:13 2013
+++ _dumbwin32proc.py.bak       Thu Sep 01 08:31:04 2011
@@ -233,17 +233,18 @@ class Process(_pollingfile._PollingTimer

         self._addPollableResource(_Reaper(self))


     def signalProcess(self, signalID):
         if self.pid is None:
             raise error.ProcessExitedAlready()
         if signalID in ("INT", "TERM", "KILL"):
-            os.popen('taskkill /T /F /PID %s' % self.pid)
+            win32process.TerminateProcess(self.hProcess, 1)
+

     def _getReason(self, status):
         if status == 0:
             return error.ProcessDone(status)
         return error.ProcessTerminated(status)


     def write(self, data):
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

5 years ago
Assignee: coop → armenzg
OS: Windows XP → All
(Assignee)

Comment 6

5 years ago
Created attachment 724494 [details] [diff] [review]
_dumbwin32proc.py
Attachment #724494 - Flags: review?(coop)
(Reporter)

Comment 7

5 years ago
Comment on attachment 724494 [details] [diff] [review]
_dumbwin32proc.py

Review of attachment 724494 [details] [diff] [review]:
-----------------------------------------------------------------

Did you check to make sure there's only one copy of _dumbwin32proc.py on each slave? We should replace it everywhere it exists to be safe.
Attachment #724494 - Flags: review?(coop) → review+
(Assignee)

Comment 8

5 years ago
(In reply to Chris Cooper [:coop] from comment #7)
> Comment on attachment 724494 [details] [diff] [review]
> _dumbwin32proc.py
> 
> Review of attachment 724494 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Did you check to make sure there's only one copy of _dumbwin32proc.py on
> each slave? We should replace it everywhere it exists to be safe.

For both win7 and winxp we only have it in here:
C:\mozilla-build\python25\Lib\site-packages\twisted\internet
C:\mozilla-build\buildbotve\Lib\site-packages\twisted\internet

On win64:
C:\mozilla-build\python\Lib\site-packages\twisted\internet
Not under C:\mozilla-build\python27
C:\mozilla-build\buildbotve\Lib\site-packages\twisted\internet

On win8:
Not under C:\mozilla-build\python27
C:\mozilla-build\buildbotve\Lib\site-packages\twisted\internet

AFAIU, we only want to replace the _dumbwin32proc.py that is used by buildbot.
(Assignee)

Comment 9

5 years ago
Comment on attachment 724494 [details] [diff] [review]
_dumbwin32proc.py

Deployed.

I have marked machines xp-0[01-10] to receive this package.
Attachment #724494 - Flags: checked-in+
(Assignee)

Comment 10

5 years ago
Created attachment 725099 [details] [diff] [review]
deploy_dumbwin32proc.bat
Attachment #725099 - Flags: review?(coop)
(Assignee)

Comment 11

5 years ago
This can be deployed like this:
C:\mozilla-build\wget\wget.exe -Odeploy_dumbwin32proc.bat https://bugzilla.mozilla.org/attachment.cgi?id=725099
deploy_dumbwin32proc.bat

I have deployed this change to:
* talos-r3-w7-012
* t-w864-ix-012
* w64-ix-slave12

I will review it tomorrow.
(Reporter)

Updated

5 years ago
Attachment #725099 - Flags: review?(coop) → review+
(Assignee)

Comment 12

5 years ago
I marked all XP slaves to get this change.
I have deployed this change to all win7 slaves as well besides:
* talos-r3-w7-006
* talos-r3-w7-011

I will wait for win64 slaves on Monday and for win8 machines for when they have ssh.
Depends on: 782177, 740496, 792456
(Assignee)

Updated

5 years ago
Blocks: 851505
(Assignee)

Comment 13

5 years ago
All remaining Windows 7 machines have already got done.

For win64 machines as Administrator (all done):
C:\mozilla-build\wget\wget.exe -q -Odeploy_dumbwin32proc.bat https://bugzilla.mozilla.org/attachment.cgi?id=725099
deploy_dumbwin32proc.bat

Win8 is left. I will see if I can ask Q to deploy it through GPO.
(Assignee)

Updated

5 years ago
Depends on: 853609
(Assignee)

Updated

5 years ago
No longer depends on: 782177
(Assignee)

Comment 15

5 years ago
Both Win64 TS and Win8 GPO got done by Q.

I adjusted the "deploy new _dumbwin32proc" section:
https://wiki.mozilla.org/ReferencePlatforms/Win64#Deploy_dumbwin32proc.py
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago5 years ago
Resolution: --- → FIXED
For the record, I would have liked to see this done with either a monkeypatch in the Buildbot source or as part of the Buildbot install process, rather than a later patch.  But I'm glad it's fixed at any rate.

Another Windows user at the PyCon sprints was talking about this problem.  Yet nobody can come up with a general solution :(
(Assignee)

Updated

5 years ago
No longer depends on: 740496
Product: mozilla.org → Release Engineering
Component: Platform Support → Buildduty
Product: Release Engineering → Infrastructure & Operations
You need to log in before you can comment on or make changes to this bug.