Win8 required mozharness changes

RESOLVED FIXED

Status

P1
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: armenzg, Assigned: armenzg)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

6 years ago
There are patches in multiple bugs.
I will dupe them forward to this bug and make it easier to follow.

For example bug 839052 and bug 840926.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 840926
(Assignee)

Updated

6 years ago
Duplicate of this bug: 839052
(Assignee)

Comment 3

6 years ago
I see this line and I still see unit tests working well.
Should I worry about it?
07:57:19  WARNING - Can't copy c:\mozilla-build\python27\python27.dll to C:\slave\test\build\venv\Scripts\python27.dll: [Errno 2] No such file or directory: 'c:\\mozilla-build\\python27\\python27.dll'!
(In reply to Armen Zambrano G. [:armenzg] from comment #3)
> I see this line and I still see unit tests working well.
> Should I worry about it?
> 07:57:19  WARNING - Can't copy c:\mozilla-build\python27\python27.dll to
> C:\slave\test\build\venv\Scripts\python27.dll: [Errno 2] No such file or
> directory: 'c:\\mozilla-build\\python27\\python27.dll'!

This is required for some windows platforms because of a bad python install.
You'll know if it's a problem because python will be busted.

In the glorious future, we'll have good python installs on all windows machines, and we can get rid of this.

http://hg.mozilla.org/build/mozharness/file/87813aad3de4/mozharness/base/python.py#l253
https://bugzilla.mozilla.org/show_bug.cgi?id=700415#c46 for the error message when you have a busted python + no dll copy.
(Assignee)

Comment 6

6 years ago
Created attachment 719070 [details] [diff] [review]
win8 mozharness changes

What do you think so far?

I just pulled an Xp and win7 slave to staging as getting results on Ash is taking forever.
Attachment #719070 - Flags: feedback?(aki)
Comment on attachment 719070 [details] [diff] [review]
win8 mozharness changes

> -    "virtualenv_python_dll": 'c:/mozilla-build/python27/python27.dll',

This will result in a busted xp? run, because xp's python wasn't installed for all users.

You should add it back, perhaps with

"virtualenv_python_dll": os.path.join(os.path.dirname(sys.executable), "python27.dll")

which I think is easier than reinstalling all xp pythons.
Attachment #719070 - Flags: feedback?(aki) → feedback+
(Assignee)

Comment 8

6 years ago
Created attachment 719183 [details] [diff] [review]
do not call winapi and win32con until we're sure we need it

I don't think this is sufficient since sys.executable would not work for win7/winxp.

If I have this code:
   sys.executable, "../scripts/external_tools/mouse_and_screen_resolution.py",
then the win7/winxp machines will resolve to C:\mozilla-build\python27\python.exe which does not have win32api.

If I do this:
   "C:\\mozilla-build\\python25\\python.exe", "../scripts/external_tools/mouse_and_screen_resolution.py",

I also filed bug 845973 in hopes to get rid of the dependency all together.

Can I install win32api inside of the virtualenv?
Flags: needinfo?(aki)
Oh, then use the venv python.
[os.path.join(os.getcwd(), "build", "venv", "...", "python"), "../scripts/external_tools/..."]

where the first '...' is scripts or wherever python goes in a windows venv.
Flags: needinfo?(aki)
(Assignee)

Comment 10

6 years ago
(In reply to Aki Sasaki [:aki] from comment #5)
> https://bugzilla.mozilla.org/show_bug.cgi?id=700415#c46 for the error
> message when you have a busted python + no dll copy.

I run this; should I worry about the dll not being copied?:
C:\Users\cltbld.T-W864-IX-001>c:/mozilla-build/python27/python c:/mozilla-build/
buildbotve/virtualenv.py --no-site-packages --distribute c:\\slave\\test\\build\
\venv
New python executable in c:\\slave\\test\\build\\venv\Scripts\python.exe
Installing distribute...........................................................
................................................................................
...........................................done.
(Assignee)

Comment 11

6 years ago
Created attachment 719535 [details] [diff] [review]
win8 mozharness changes

This is what I'm now testing which so far has looked good.
I am now running opt, debug and talos jobs for win7, winxp and win8.
I will ask a review after that (optimistically in the next couple of hours).

Once all three run well, I will do a sendchange to trigger all unit tests to make sure I don't have a loose end.
Attachment #719070 - Attachment is obsolete: true
Attachment #719183 - Attachment is obsolete: true
Comment on attachment 719535 [details] [diff] [review]
win8 mozharness changes

Awesome.
You could set VIRTUALENV_PATH = os.path.join(os.getcwd(), 'build', 'venv') and use that every place you're referencing it (virtualenv_path, the dll, mozinstall, and the mouse_and_screen_resolution call), which would be cleaner.

However, if this passes, I'm happy to have it land as is.
(Assignee)

Comment 13

6 years ago
(In reply to Aki Sasaki [:aki] from comment #12)
> Comment on attachment 719535 [details] [diff] [review]
> win8 mozharness changes
> 
> Awesome.
> You could set VIRTUALENV_PATH = os.path.join(os.getcwd(), 'build', 'venv')
> and use that every place you're referencing it (virtualenv_path, the dll,
> mozinstall, and the mouse_and_screen_resolution call), which would be
> cleaner.
> 
> However, if this passes, I'm happy to have it land as is.

I'm not having luck. python27 and venv's python on Windows 7 do not have win32api and we cannot install it through pip install.
I should give you a hand. I've had luck installing pywin32 via virtualenv_modules.
(Assignee)

Comment 15

6 years ago
Created attachment 719702 [details] [diff] [review]
win8 mozharness changes

Yay!
Attachment #719535 - Attachment is obsolete: true
Attachment #719702 - Flags: review?(aki)
Comment on attachment 719702 [details] [diff] [review]
win8 mozharness changes

>--- a/scripts/desktop_unittest.py
>+++ b/scripts/desktop_unittest.py
>@@ -63,16 +63,17 @@ class DesktopUnittest(TestingMixin, Merc
>             "help": "This will run all suites that are specified "
>                     "in the config file. You do not need to specify "
>                     "any other suites.\nBeware, this may take a while ;)"}
>          ],
>     ] + copy.deepcopy(testing_config_options)
> 
>     virtualenv_modules = [
>         "simplejson",
>+        "pywin32",
>         {'mozlog': os.path.join('tests', 'mozbase', 'mozlog')},
>         {'mozinfo': os.path.join('tests', 'mozbase', 'mozinfo')},
>         {'mozhttpd': os.path.join('tests', 'mozbase', 'mozhttpd')},
>         {'mozfile': os.path.join('tests', 'mozbase', 'mozfile')},
>         {'mozinstall': os.path.join('tests', 'mozbase', 'mozinstall')},
>         {'manifestdestiny': os.path.join('tests', 'mozbase', 'manifestdestiny')},
>         {'mozprofile': os.path.join('tests', 'mozbase', 'mozprofile')},
>         {'mozprocess': os.path.join('tests', 'mozbase', 'mozprocess')},

This'll break Linux and OSX.
r=me if you remove the pywin32 here, and copy this entire virtualenv_modules (with pywin32) into win_config.py.
I don't like the duplication, but that's the fix here.

And thank you for working through this Armen!
Attachment #719702 - Flags: review?(aki) → review+
(Assignee)

Comment 17

6 years ago
Created attachment 720079 [details] [diff] [review]
win8 mozharness changes + no need for pywin32

No need for pywin32.
Attachment #719702 - Attachment is obsolete: true
Attachment #720079 - Flags: review?(aki)
Comment on attachment 720079 [details] [diff] [review]
win8 mozharness changes + no need for pywin32

I'm not going to ask how you figured out the mouse_and_screen_resolution.py changes, but cool.
Attachment #720079 - Flags: review?(aki) → review+
(Assignee)

Comment 20

6 years ago
Comment on attachment 720079 [details] [diff] [review]
win8 mozharness changes + no need for pywin32

Landed and merged:
http://hg.mozilla.org/build/mozharness/rev/1e80b047942f
Attachment #720079 - Flags: checked-in+
(Assignee)

Comment 21

6 years ago
This log shows that the new mouse_and_screen_resolution.py is being used:
https://tbpl.mozilla.org/php/getParsedLog.php?id=20297332&tree=Cedar&full=1
https://tbpl.mozilla.org/?tree=Cedar&jobname=Rev3 WINNT 6.1 cedar opt test mochitest-5&rev=929c16dc4380 (2nd job)
http://hg.mozilla.org/build/mozharness/rev/1e80b047942f

I'm testing a 2 line patch before I close this bug:
http://hg.mozilla.org/users/armenzg_mozilla.com/mozharness/rev/090d161e8a5c
(Assignee)

Updated

6 years ago
Blocks: 731280
No longer blocks: 844130
Summary: Address win8 required mozharness changes → Win8 required mozharness changes
(Assignee)

Comment 22

6 years ago
Created attachment 720766 [details] [diff] [review]
Two lines change
(Assignee)

Updated

6 years ago
Blocks: 826050
No longer blocks: 731280
(Assignee)

Comment 23

6 years ago
Comment on attachment 720766 [details] [diff] [review]
Two lines change

It went well.
Attachment #720766 - Attachment description: [being tested] Two lines change → Two lines change
Attachment #720766 - Flags: review?(aki)
Attachment #720766 - Flags: review?(aki) → review+
(Assignee)

Comment 24

6 years ago
Comment on attachment 720766 [details] [diff] [review]
Two lines change

dbc652240eb8
Attachment #720766 - Flags: checked-in+
(Assignee)

Comment 25

6 years ago
Merged 16 hours ago:
http://hg.mozilla.org/build/mozharness/rev/3f4dbe4f9b4c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Component: General Automation → Mozharness
You need to log in before you can comment on or make changes to this bug.