844982 - Win8 required mozharness changes

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect, P1)

Description

[Armen [:armenzg]]

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.

Comment 3

[Armen [:armenzg]]

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'!

Comment 4

[Aki Sasaki (not active)]

(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

Comment 5

[Aki Sasaki (not active)]

https://bugzilla.mozilla.org/show_bug.cgi?id=700415#c46 for the error message when you have a busted python + no dll copy.

Comment 6

[Armen [:armenzg]]

Attachment: 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.

Comment 7

[Aki Sasaki (not active)]

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.

Comment 8

[Armen [:armenzg]]

Attachment: 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?

Comment 9

[Aki Sasaki (not active)]

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.

Comment 10

[Armen [:armenzg]]

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

Comment 11

[Armen [:armenzg]]

Attachment: 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.

Comment 12

[Aki Sasaki (not active)]

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.

Comment 13

[Armen [:armenzg]]

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

Comment 14

[Aki Sasaki (not active)]

I should give you a hand. I've had luck installing pywin32 via virtualenv_modules.

Comment 15

[Armen [:armenzg]]

Attachment: win8 mozharness changes

Yay!

Comment 16

[Aki Sasaki (not active)]

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!

Comment 17

[Armen [:armenzg]]

Attachment: win8 mozharness changes + no need for pywin32

No need for pywin32.

Comment 18

[Armen [:armenzg]]

I see green on Ash:
https://tbpl.mozilla.org/?tree=Ash&jobname=WINNT&rev=07e3c1fa05c6

Comment 19

[Aki Sasaki (not active)]

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.

Comment 20

[Armen [:armenzg]]

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

Comment 21

[Armen [:armenzg]]

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

Comment 22

[Armen [:armenzg]]

Attachment: Two lines change

Comment 23

[Armen [:armenzg]]

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

It went well.

Comment 24

[Armen [:armenzg]]

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

dbc652240eb8

Comment 25

[Armen [:armenzg]]

Merged 16 hours ago:
http://hg.mozilla.org/build/mozharness/rev/3f4dbe4f9b4c