Jeff Hammel Reporter
	Description • 12 years ago

17:20:33     INFO - Installing talos into virtualenv
/d/Objects/mozharness/tegra_bwd\build\venv
17:20:33     INFO - Running command:
['d:\\d\\Objects\\mozharness\\tegra_bwd\\build\\venv\\Scripts\\
pip', 'install', '--pypi-url',
'http://people.mozilla.com/~jhammel/pypi/', 'http://hg.mozilla.org/bu
ild/talos/archive/tip.tar.gz'] in
/d/Objects/mozharness/tegra_bwd\build
17:20:50     INFO -  Downloading/unpacking
http://hg.mozilla.org/build/talos/archive/tip.tar.gz
17:20:50     INFO -    Running setup.py egg_info for package from
http://hg.mozilla.org/build/talos/
archive/tip.tar.gz
17:20:50     INFO -  Downloading/unpacking PyYAML (from talos==0.0)
17:20:50     INFO -    Running setup.py egg_info for package PyYAML
17:20:50     INFO -  Downloading/unpacking mozdevice>=0.2 (from
talos==0.0)
17:20:50     INFO -    Downloading mozdevice-0.2.tar.gz
17:20:50     INFO -    Running setup.py egg_info for package mozdevice
17:20:50     INFO -  Downloading/unpacking mozhttpd>=0.3 (from
talos==0.0)
17:20:50     INFO -    Downloading mozhttpd-0.3.tar.gz
17:20:50     INFO -    Running setup.py egg_info for package mozhttpd
17:20:50     INFO -  Downloading/unpacking mozinfo (from talos==0.0)
17:20:50     INFO -    Downloading mozinfo-0.3.3.tar.gz
17:20:50     INFO -    Running setup.py egg_info for package mozinfo
17:20:50     INFO -  Downloading/unpacking pywin32 (from talos==0.0)
17:20:50     INFO -    Could not find any downloads that satisfy the
requirement pywin32 (from talos
==0.0)
17:20:50     INFO -  No distributions at all found for pywin32 (from
talos==0.0)
17:20:50     INFO -  Storing complete log in
C:\Users\Justin\AppData\Roaming\pip\pip.log
17:20:50    ERROR - Return code: 1
17:20:50    FATAL - Unable to install
http://hg.mozilla.org/build/talos/archive/tip.tar.gz!
17:20:50    FATAL - Exiting -1

 Talos adds the pywin32 dependency via a definitive link since pywin32 isn't
 available on pypi:

http://hg.mozilla.org/build/talos/file/364f3167176e/setup.py#l16

It appears that pip --pypi-url does not follow this link and bails out
instead :( This is not the behaviour I would expect.  The link could
also be bad (I haven't checked).  Talos should probably print out the
link for debugging.

There are several ways around this problem, but it is somewhat
annoying for the mozharness production case

	Jeff Hammel Reporter
	Comment 1 • 12 years ago

It has come to my attention that installing pywin32 on the system in question does not work, so it is probably not a pip eccentricity

	Justin Wood (:Callek)
	Comment 2 • 12 years ago

for what its worth, this is my local win7 system, and to install pywin32 for this, what I did was:

* Run talosrunner.py --create-virtualenv
** Failed due to pywin32
* in a *new* bash environ with UAC-Administrator: build/venv/scripts/easy_install -Z ../pywin32*.exe
** the UAC was needed otherwise I got "Bad file number" due to "install" in easy_install's name.
* Re-Ran the talosrunner command.

	Aki Sasaki (not active) Assignee
	Comment 3 • 12 years ago

This is not pretty, but this worked:

build\venv\scripts\python build\venv\scripts\easy_install-2.7-script.py http://people.mozilla.org/~asasaki/pywin32-216.win32-py27.exe

That avoided the UAC since the executable is named python.

I think I can get that into the talos create-virtualenv action with some hackery.

	Aki Sasaki (not active) Assignee
	Comment 4 • 12 years ago

(In reply to Aki Sasaki [:aki] from comment #3)
> This is not pretty, but this worked:
> 
> build\venv\scripts\python build\venv\scripts\easy_install-2.7-script.py
> http://people.mozilla.org/~asasaki/pywin32-216.win32-py27.exe

With this patch, you can:

a) add pywin32 to self.config['virtualenv_modules'], before talos,
b) specify self.config['pywin32_url'], and
c) specify self.config['exes']['easy_install'] to point to ['VENV\scripts\python', 'VENV\scripts\easy_install-2.7-script.py']

I tested on a win7 staging test slave and got past the create-virtualenv action in talos_script with my config-file-in-progress.

Now I'm dying on "mozinstall" requiring elevated permissions.  I think I can get around that by prepending the appropriate python as I did here.

	Aki Sasaki (not active) Assignee
	Comment 5 • 12 years ago

Same reasoning and solution as the previous patch.
Here I set self.config['exes']['mozinstall'] to ['%s/scripts/python' % VENV_PATH, '%s/scripts/mozinstall-script.py' % VENV_PATH]

	Aki Sasaki (not active) Assignee
	Comment 6 • 12 years ago

We can't check for os.path.exists(self.query_python_path("PerfConfigurator")) because on Windows the command name doesn't need a .exe to run, but requires the .exe to check for existence.  Same with talos.

I'm just removing these checks because:

a) we self.fatal() after the perfconfigurator call anyway, if it was unsuccessful.
b) we set self.return_code to the talos command's return status.  In downstream scripts like jetperf, we can add a postflight_run_tests() that self.fatal()s if desired.

The alternate fix is to add the .exe if it's Windows, but I think this is cleaner and allows you to see the command line run even if it fails.  It should fail pretty quick if they don't exist.


At this point, I'm getting all the way to the run-tests action.
It's failing because the mozinstall call is outputting 'None' for our binary_path.  I may have to just hardcode WORK_DIR\application\firefox\firefox.exe as the binary_path in our windows config file if we can't get mozinstall to work the way we want.

	Jeff Hammel Reporter
	Comment 7 • 12 years ago

Comment on attachment 633333 [details] [diff] [review]
allow for easy_install in install_module()

I am still confused on exactly why this fails with pip but haven't had a chance to look at it.  This works for me

	Aki Sasaki (not active) Assignee
	Comment 8 • 12 years ago

Comment on attachment 633333 [details] [diff] [review]
allow for easy_install in install_module()

http://hg.mozilla.org/build/mozharness/rev/9da873c20a93

	Jeff Hammel Reporter
	Comment 9 • 12 years ago

(In reply to Aki Sasaki [:aki] from comment #6)

> At this point, I'm getting all the way to the run-tests action.
> It's failing because the mozinstall call is outputting 'None' for our
> binary_path.  I may have to just hardcode
> WORK_DIR\application\firefox\firefox.exe as the binary_path in our windows
> config file if we can't get mozinstall to work the way we want.

Do we know why this is?  Could this be a mozinstall bug?

	Jeff Hammel Reporter
	Comment 10 • 12 years ago

Comment on attachment 633353 [details] [diff] [review]
fix perfconfigurator + talos

This is fine.  I'd prefer a better solution but.... :/

	Jeff Hammel Reporter
	Comment 11 • 12 years ago

Comment on attachment 633345 [details] [diff] [review]
fix mozinstall

This is fine.  I'm not a big fan of seeing logic duplicated, e.g.

foo = self.query_exe('foo', default=self.query_python_path('foo'))
if isinstance(foo, list):
    cmd = foo[:] # have to copy since you don't want to modify the instance exe_dict value
else:
    cmd = [foo]

Maybe worth putting in a convenience method?  (e.g. query_python_command)

Other than that, looks fine.

	Aki Sasaki (not active) Assignee
	Comment 12 • 12 years ago

(In reply to Jeff Hammel [:jhammel] from comment #9)
> (In reply to Aki Sasaki [:aki] from comment #6)
> 
> > At this point, I'm getting all the way to the run-tests action.
> > It's failing because the mozinstall call is outputting 'None' for our
> > binary_path.  I may have to just hardcode
> > WORK_DIR\application\firefox\firefox.exe as the binary_path in our windows
> > config file if we can't get mozinstall to work the way we want.
> 
> Do we know why this is?  Could this be a mozinstall bug?

Quite possibly, though I don't know what the expected behavior is.
It would be nice to output the binary path.

	Aki Sasaki (not active) Assignee
	Comment 13 • 12 years ago

Comment on attachment 633345 [details] [diff] [review]
fix mozinstall

http://hg.mozilla.org/build/mozharness/rev/748f73a41155

	Aki Sasaki (not active) Assignee
	Comment 14 • 12 years ago

Comment on attachment 633353 [details] [diff] [review]
fix perfconfigurator + talos

http://hg.mozilla.org/build/mozharness/rev/361b5d44f0a3

	Aki Sasaki (not active) Assignee
	Comment 15 • 12 years ago

(In reply to Jeff Hammel [:jhammel] from comment #11)
> Comment on attachment 633345 [details] [diff] [review]
> fix mozinstall
> 
> This is fine.  I'm not a big fan of seeing logic duplicated, e.g.
> 
> foo = self.query_exe('foo', default=self.query_python_path('foo'))
> if isinstance(foo, list):
>     cmd = foo[:] # have to copy since you don't want to modify the instance
> exe_dict value
> else:
>     cmd = [foo]
> 
> Maybe worth putting in a convenience method?  (e.g. query_python_command)

Yeah.  Either that, or

foo = self.query_exe('foo', default=self.query_python_path('foo'), return_type="list")

?

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 16 • 12 years ago

(In reply to Jeff Hammel [:jhammel] from comment #9)
> Do we know why this is?  Could this be a mozinstall bug?

Is this solved meanwhile or which command have you issued to run into the problem with mozinstall?

	Aki Sasaki (not active) Assignee
	Comment 17 • 12 years ago

(In reply to Henrik Skupin (:whimboo) from comment #16)
> (In reply to Jeff Hammel [:jhammel] from comment #9)
> > Do we know why this is?  Could this be a mozinstall bug?
> 
> Is this solved meanwhile or which command have you issued to run into the
> problem with mozinstall?

See above.
I'm calling

['%s/scripts/python' % VENV_PATH, '%s/scripts/mozinstall-script.py' % VENV_PATH] on Windows.
It doesn't output anything, so I'm going to have to guess where the binary is.

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 18 • 12 years ago

Well, please tell me what's getting called here exactly with filled-in content for the placeholders.

	Jeff Hammel Reporter
	Comment 19 • 12 years ago

This sounds like a bug to me

	Aki Sasaki (not active) Assignee
	Comment 20 • 12 years ago

(In reply to Henrik Skupin (:whimboo) from comment #18)
> Well, please tell me what's getting called here exactly with filled-in
> content for the placeholders.

18:25:31     INFO - Getting output from command: ['c:/talos-slave/test/build/venv/scripts/python', 'c:/talos-slave/test/build/venv/scripts/mozinstall-script.py', '--source', 'C:\\talos-slave\\test\\build\\firefox-16.0a1.en-US.win32.zip', '--destination', 'C:\\talos-slave\\test\\build\\application']
18:25:32     INFO - Reading from file tmpfile_stdout
18:25:32     INFO - Output received:
18:25:32     INFO -  None

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 21 • 12 years ago

Have you tried with MozInstall 1.0 yet? If not please obey that there is no --source option anymore. It's a mandatory argument now. Only --destination remains as option.

	Aki Sasaki (not active) Assignee
	Comment 22 • 12 years ago

(In reply to Henrik Skupin (:whimboo) from comment #21)
> Have you tried with MozInstall 1.0 yet? If not please obey that there is no
> --source option anymore. It's a mandatory argument now. Only --destination
> remains as option.

mozinstall 1.0 works.

Now I need to put in a conditional '-s' depending if we're running the mozinstall 1.0 from the pypi server, or the test zip mozinstall [0.3?] from the tree.

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 23 • 12 years ago

Shouldn't we then better land mozinstall 1.0 in mozilla-central?

	Aki Sasaki (not active) Assignee
	Comment 24 • 12 years ago

(In reply to Henrik Skupin (:whimboo) from comment #23)
> Shouldn't we then better land mozinstall 1.0 in mozilla-central?

Yes. I don't want to block on that though, especially since anything that uses mozinstall in m-c is going to have to change their command line options.

	Jeff Hammel Reporter
	Comment 25 • 12 years ago

(In reply to Henrik Skupin (:whimboo) from comment #23)
> Shouldn't we then better land mozinstall 1.0 in mozilla-central?

ticketed: https://bugzilla.mozilla.org/show_bug.cgi?id=766759

	Aki Sasaki (not active) Assignee
	Comment 26 • 12 years ago

with query_exe change to allow for return_type="list".

I've gone as far as launching firefox to run ts with this patch, after which it gacks on the graphserver file:///c:\path\to\talos.txt .  I figure that, plus all the config that needs to be passed to PerfConfigurator in production, belongs in a different bug.

	Jeff Hammel Reporter
	Comment 27 • 12 years ago

Comment on attachment 635119 [details] [diff] [review]
allow for mozinstall 0.3 or 1.0

+        if return_type == "list":
+            if not isinstance(exe, list):
+                exe = list(exe)

So if it is not a list, what will it be?  If a string, then this is wrong:

>>> list("echo")
['e', 'c', 'h', 'o']

+            if not isinstance(exe, str):
+                # XXX Requesting return_type of "string" with
+                # spaces/unintentionally unescaped special chars is just
+                # going to cause you unhappiness.  If there's a chance of
+                # that, use a list.
+                exe = ' '.join(exe)
+                self.warning("Returning a concatenated string %s for %s.\nThis can cause issues if there are spaces or unescaped special characters." % (exe, exe_name))

This is what subprocess.list2cmdline is for:

>>> subprocess.list2cmdline(['echo', 'a long string with spaces and "" in it', '--foo'])
'echo "a long string with spaces and \\"\\" in it" --foo'

	Aki Sasaki (not active) Assignee
	Comment 28 • 12 years ago

Good catch.
I'm making str and list the only allowable types by default, though someone could override that by calling query_exe with an error_level lower than FATAL.
Not entirely sure if/when that would happen.

	Jeff Hammel Reporter
	Comment 29 • 12 years ago

Comment on attachment 635524 [details] [diff] [review]
addressing review comments

looks good to me

	Aki Sasaki (not active) Assignee
	Comment 30 • 12 years ago

Comment on attachment 635524 [details] [diff] [review]
addressing review comments

http://hg.mozilla.org/build/mozharness/rev/6d12394f6bc7

	Aki Sasaki (not active) Assignee
	Comment 31 • 12 years ago

I think the rest has to do with adding the right perfconfigurator options, which I'll track in bug 760320.