911048 - [mozinstall] Add support to install Firefox via the stub installer

Status: NEW

(Testing :: Mozbase, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As given by bug 799406 the stub installer does not have an unattended (silent) mode, and will not get one in the future. That makes it hard for us to get a build of Firefox installed. We might want to look into possible solutions how this can be accomplished. As what I have found Autoit (http://www.autoitscript.com/site/autoit/) could help us here by simply clicking through the installer. We even can create an executable for it, so it should be easy to ship mozinstall with it.

Comment 1

[Jarek Śmiejczak [:jotes]]

If this is required only for windows we can try to use: http://code.google.com/p/pywinauto/

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Oh, good hint, Jarek! And it's even available on pypi: https://pypi.python.org/pypi/pywinauto/0.4.0

Jeff, what do you think about that tool? I think that's a more elegant solution if it works with the stub installer for Windows.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Well, I should at least CC or ask for information from Jeff.

Comment 4

[Jeff Hammel]

(In reply to Henrik Skupin (:whimboo) from comment #2)
> Oh, good hint, Jarek! And it's even available on pypi:
> https://pypi.python.org/pypi/pywinauto/0.4.0
> 
> Jeff, what do you think about that tool? I think that's a more elegant
> solution if it works with the stub installer for Windows.

I'm not really an expert in this problem space; it looks fine at first glance, but again, I can't offer an expert opinion.

Comment 5

[Jarek Śmiejczak [:jotes]]

So, after our talk on irc, i've started working on this bug. Results: i'm using pywinauto and i can automate whole instalation process but...
To identify elements i'm searching them in following manner:
https://gist.github.com/jarekps/6544268

It's easy to automate one version of app, but as i suppose we should have similar strings for other locales (than en.US) to identify elements... Are there any other localizations than en-US at the moment?

Also, i have strange issue with stub installer. It looks like stub installer doesn't work with our __compat_layer and executes new child process inside which causes UAC window (which requires user's interaction), when you haven't run console with admin privileges.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Jarek Śmiejczak from comment #5)
> It's easy to automate one version of app, but as i suppose we should have
> similar strings for other locales (than en.US) to identify elements... Are
> there any other localizations than en-US at the moment?

That looks like you are using labels here. Don't have those elements unique ids we can make use of?

> Also, i have strange issue with stub installer. It looks like stub installer
> doesn't work with our __compat_layer and executes new child process inside
> which causes UAC window (which requires user's interaction), when you
> haven't run console with admin privileges.

Doesn't that also apply to the normal installer? On our testing machines we have turned off UAC because of that.

Comment 7

[Jarek Śmiejczak [:jotes]]

Attachment: 0001-Bug-911048-Support-for-installation-via-stub-install.patch

> That looks like you are using labels here. Don't have those elements unique ids we can make use of?
Ok, I've changed my strategy a little bit thanks to our talk on irc.

>Doesn't that also apply to the normal installer? On our testing machines we have turned off UAC because of that.
I've added waiting for window with big timeout to avoid hangup if window is not found or user's slow reaction.

It's a initial version of patch. I've tested it on recent nightlies of firefox.
I hope it will serve it's purpose.

Comment 8

[Jarek Śmiejczak [:jotes]]

Attachment: 0001-Bug-911048-Support-for-installation-via-stub-install.patch

Updated version with passing tests.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 804937 [details] [diff] [review]
0001-Bug-911048-Support-for-installation-via-stub-install.patch

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

Looks that this can be done kinda easily. It's a nice solution proposed here. I like it. I have made a couple of comments you can find inline. Also I wonder if we should create a sub module for all the pyauto code. That wouldn't clutter the main mozinstall module.

::: mozinstall/mozinstall/mozinstall.py
@@ +120,4 @@
>          elif src.lower().endswith('.dmg'):
>              install_dir = _install_dmg(src, dest)
>          elif src.lower().endswith('.exe'):
> +            if src.lower().endswith('stub.exe'):

I wouldn't rely on that we will always end with 'stub.exe'. Can't we simply check if ´stub´ is contained? Or is there a special file info available via the pefile module?

@@ +122,5 @@
>          elif src.lower().endswith('.exe'):
> +            if src.lower().endswith('stub.exe'):
> +                install_dir = _install_stub(src, dest, app_name)
> +            else:
> +                install_dir = _install_exe(src, dest)

I would call it ´_install_win_full()´

@@ +284,5 @@
> +def _get_process_window(process_name, timeout=120.):
> +    """Finds main window of found process.
> +
> +    Arguments:
> +    process_name -- name of process 

nit: trailing blank

@@ +288,5 @@
> +    process_name -- name of process 
> +    timeout - a number of seconds to wait for main window.
> +    """
> +    from pywinauto import application
> +    from pywinauto import findwindows

We want to import it globally at the top of the file.

@@ +300,5 @@
> +
> +        except (findwindows.WindowNotFoundError, IndexError):
> +            pass
> +
> +    raise Exception("Couldn't find main window of application.")

Can we get added the process name to the exception message?

@@ +303,5 @@
> +
> +    raise Exception("Couldn't find main window of application.")
> +
> +
> +def _wait_for_window_close(window):

It's used once, so I don't think we need this as an extra method.

@@ +310,5 @@
> +    Arguments:
> +    window -- instance of pywinauto's window
> +    """
> +    while window.Exists():
> +        time.sleep(1)

I would do shorter sleeps. Also when do we abort?

@@ +332,5 @@
> +    install_window['Button0'].Click()
> +
> +    # Edit - Destination directory input
> +    install_window['Edit'].TypeKeys('^a') # Marks all text inside input
> +    install_window['Edit'].TypeKeys(dest) 

nit: trailing blank

@@ +340,5 @@
> +
> +    _wait_for_window_close(install_window)
> +
> +    # Installers opens fresh installed app which we want to close
> +    app_pid, _ = _get_process_window('%s.exe' % app_name)

This will most likely not work with XulRunner builds, but I'm not aware that we will get stub installers for it. Btw. is there no checkbox we could uncheck? I don't think it's a good idea to let the installer start the application - even when we can close it immediately.

::: mozinstall/setup.py
@@ +15,5 @@
>  
>  deps = ['mozinfo >= 0.4',
> +        'mozfile',
> +        'mozprocess >= 0.11',
> +        'pywinauto==0.4.1'

nit: blanks around the operator. It will conflict with the other patch for the in_installer() check.

Comment 10

[Jarek Śmiejczak [:jotes]]

> This will most likely not work with XulRunner builds, but I'm not aware that we will get stub installers for it. Btw. is there no checkbox we could uncheck? I don't think it's a good idea to let the installer start the application - even when we can close it immediately.

Unfortunately, for current state, stub installer doesn't provide such option (to abort running program after install), that's why i'm killing process after start.
I can remove this line, with explicit wait for end of download and just exit from mozinstall when download begins.

Also i've discovered strange issue on Windows x64 - mozprocess doesn't list some processes (e.g. children of stub-setup.exe) in get_pids what causes error in find_window function. Sorry for no updated today but i must investigate this a little bit longer to find reasonable solution.

Comment 11

[Jarek Śmiejczak [:jotes]]

Attachment: 0001-Bug-911048-Support-for-installation-via-stub-install.patch

Patch updated (i've implemented previous comments).

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 807491 [details] [diff] [review]
0001-Bug-911048-Support-for-installation-via-stub-install.patch

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

::: mozinstall/mozinstall/mozinstall.py
@@ +12,4 @@
>  from optparse import OptionParser
>  import os
> +from pywinauto import application
> +from pywinauto import findwindows

Can we please get those two lines added to a separate block given that those are not modules shipped by default?

@@ +101,4 @@
>      src  -- the path to the install file
>      dest -- the path to install to (to ensure we do not overwrite any existent
>                                      files the folder should not exist yet)
> +    app_name -- name of installed application

nit: name of application to be installed

@@ +122,4 @@
>          elif src.lower().endswith('.dmg'):
>              install_dir = _install_dmg(src, dest)
>          elif src.lower().endswith('.exe'):
> +            if src.lower().find('stub') != -1:

Does ´'stub' in src.lower()´ not work here?

@@ +124,5 @@
>          elif src.lower().endswith('.exe'):
> +            if src.lower().find('stub') != -1:
> +                install_dir = _install_win_full(src, dest, app_name)
> +            else:
> +                install_dir = _install_exe(src, dest)

I would suggest we also update this function name accordingly to the stub installer one.

@@ +292,5 @@
> +
> +    """
> +    start_time = time.time()
> +
> +    while time.time() - start_time <= timeout:

I wouldn't do the calculation each time. Set the end time right before entering the loop and just compare the current time with it.

@@ +305,5 @@
> +    raise Exception("Couldn't find main window of '%s'." % process_name)
> +
> +
> +def _install_win_full(src, dest, app_name):
> +    """Run the stub installer and automates process of installing application.

The method name doesn't match. Please call it _install_exe_stub(). Also please add that a silent installation method is not available yet, and will most likely not be.

@@ +315,5 @@
> +
> +    """
> +    install_proc = subprocess.Popen(src)
> +
> +    _, install_window = _get_process_window('setup-stub.exe')

Is setup-stub.exe always the process name? Or where does it come from?

@@ +321,5 @@
> +    # Button0 - Options button
> +    install_window['Button0'].Click()
> +
> +    # Edit - Destination directory input
> +    install_window['Edit'].TypeKeys('^a') # Marks all text inside input

I would drop the comment. Can't we simply reset the value directly via an API call?

Comment 13

[Jarek Śmiejczak [:jotes]]

Attachment: 0001-Bug-911048-Support-for-installation-via-stub-install.patch

> Is setup-stub.exe always the process name? Or where does it come from?
Yes, it's just process name. For stub installers i've tried process_name was always 'setup-stub.exe' (called as a subprocess of main-installer from a randomnly generated temporary directory). However, i'm not sure if there's available installer for different arch than win32? I've looked at latest nightlies.

I've implemented rest of your notes, please review again.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 811671 [details] [diff] [review]
0001-Bug-911048-Support-for-installation-via-stub-install.patch

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

> Yes, it's just process name. For stub installers i've tried process_name was
> always 'setup-stub.exe' (called as a subprocess of main-installer from a

Ok, I just wondered because we could trap into a situation when another stub installer is already active and we will get the wrong one. So we have to be careful here.

I tried this patch on my Win8 machine with Firefox 24.0 but it fails with:

Traceback (most recent call last):
  File "c:\mozilla\a\mozmill-env\python\Scripts\mozinstall-script.py", line 8, in <module>
    load_entry_point('mozInstall==1.7', 'console_scripts', 'mozinstall')()
  File "c:\mozilla\mozbase\mozinstall\mozinstall\mozinstall.py", line 364, in install_cli
    install_path = install(src, options.dest, options.app)
  File "c:\mozilla\mozbase\mozinstall\mozinstall\mozinstall.py", line 127, in install
    install_dir = _install_win_stub(src, dest, app_name)
  File "c:\mozilla\mozbase\mozinstall\mozinstall\mozinstall.py", line 322, in _install_win_stub
    _, install_window = _get_process_window('setup-stub.exe')
  File "c:\mozilla\mozbase\mozinstall\mozinstall\mozinstall.py", line 301, in _get_process_window
    return [window_pid, app.top_window_()]
  File "build\bdist.win32\egg\pywinauto\application.py", line 961, in top_window_
mozinstall.mozinstall.InstallError: Failed to install "c:\mozilla\mozbase\firefox-24.0.en-US.win32-stub.exe"

After checking the processes I can clearly see that 2 'setup-stub.exe' processes are running. So we most likely get the wrong PID.

Comment 15

[Jarek Śmiejczak [:jotes]]

> After checking the processes I can clearly see that 2 'setup-stub.exe' processes are running. So we most likely get the wrong PID.

We're looping through all processes with this name ('setup-stub.exe') with assumption that only one of them has available window.
I think that in this case mozinstall cannot access to process due to UAC and non-elevated state.
I'm uploading patch which propably fixes this issue. However, i'm aware that some parts of code in this patch should be applied elsewhere (like mozprocess?). It's more to checkout if my whole speculation/concept is good. So my request would be to run this code again on your machine. 
Thanks in advance!

Comment 16

[Jarek Śmiejczak [:jotes]]

Attachment: 0001-Bug-911048-Support-for-installation-via-stub-install.patch

Comment 17

[Jeff Hammel]

I agree that mozinstall is not the best place for some of the windows code.  It makes me wonder if an e.g. mozwinutils is in order (note that I and :wlach hate utils packages/modules/files, but I'm stabbing in the dark here).

Comment 18

[Jarek Śmiejczak [:jotes]]

:jhammel
Me too (i hate all god/utils like modules)! I would be glad if you somebody point me to better solution as a whole UAC thing is only windows specific. Or maybe we should assume that it's responsibility of the user to run this program (mozinstall) properly (elevate permisions via runas [windows command-line] propably).

I'm coming from posix world, i have so much ignorance :D

Comment 19

[Jeff Hammel]

(In reply to Jarek Śmiejczak from comment #18)
> :jhammel
> Me too (i hate all god/utils like modules)! I would be glad if you somebody
> point me to better solution as a whole UAC thing is only windows specific.
> Or maybe we should assume that it's responsibility of the user to run this
> program (mozinstall) properly (elevate permisions via runas [windows
> command-line] propably).
> 
> I'm coming from posix world, i have so much ignorance :D

Sorry, no _good_ advice here.  I'm also from a posix background and don't have a great intuition in this case, where this code should go.  Just butting into what probably ain't my business ;)

No, the code is fine where you have it for now , pending Henrik's review.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 817501 [details] [diff] [review]
0001-Bug-911048-Support-for-installation-via-stub-install.patch

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

This patch somewhat enters an infinite loop on my system. Command windows are getting opened and closed all the time. It's hard to get it stopped at all.

::: mozinstall/mozinstall/uac.py
@@ +38,5 @@
> +    execinfo.lpVerb = "runas"
> +    execinfo.lpFile = "mozinstall.exe"
> +    execinfo.lpParameters = '--destination="%s" "%s"' % (dst, src)
> +    execinfo.nShow = 1
> +    ShellExecuteEx(ctypes.byref(execinfo))

Where you got all this from? Any URL you can add as comment?

Comment 21

[Jarek Śmiejczak [:jotes]]

Attachment: 0001-Bug-911048-Support-for-installation-via-stub-install.patch

Okay, i've added patch which doesn't contain this UAC forcing python.
After applying this patch, there are 2 ways which provides us enough permissions to run stub installer:

1. Disable UAC on your machine (there are plenty of tutorials on internet about this) (inconvienient)
2. Execute commandline by clicking "Run As Administrator" on cmd.exe and run mozinstall inside (easiest, fastest)

In other ways, windows will display UAC confirmation window and make mozinstall fail.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 823083 [details] [diff] [review]
0001-Bug-911048-Support-for-installation-via-stub-install.patch

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

Sorry, for the delay in review. Meanwhile the patch doesn't apply cleanly anymore. So we need another update.

(In reply to Jarek Śmiejczak from comment #21)
> 1. Disable UAC on your machine (there are plenty of tutorials on internet
> about this) (inconvienient)
> 2. Execute commandline by clicking "Run As Administrator" on cmd.exe and run
> mozinstall inside (easiest, fastest)
> 
> In other ways, windows will display UAC confirmation window and make
> mozinstall fail.

Jarek, is there a way to get the information up-front if the script has the right permissions to request this action? I'm worried that it would stall with the UAC prompt. It would be better to error out earlier with an appropriate exception.

::: mozinstall/mozinstall/mozinstall.py
@@ +283,4 @@
>      return dest
>  
>  
> +def _get_process_window(process_name, timeout=120.):

Should this method be something for mozprocess? I feel its better suited over there.

Comment 23

[Jarek Śmiejczak [:jotes]]

> Jarek, is there a way to get the information up-front if the script has the right permissions to request this action?

I can add such information at the start of script. Will be added in next patch.

> Should this method be something for mozprocess?
I don't feel so. It's more helper for pywinauto to find proper window. I think it's more material for separate package than something closely related for mozprocess. And it would be supported only on windows.

I'll upload changes later today. I had a little delay caused by ConEmu bug.

Comment 24

[Jarek Śmiejczak [:jotes]]

Attachment: 0001-Bug-911048-Support-for-installation-via-stub-install.patch

I've added checking if user has rights of administrator. Please review again.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Given the external dependencies we have for this patch I'm seeking for Andrew's feedback. This is an important feature we need for mozinstall to be able to install Firefox via the stub installer. So what is your take Andrew? Shall we move this as a general discussion about external deps to the tools newsgroup?

Comment 26

[Andrew Halberstadt [:ahal]]

(In reply to Henrik Skupin (:whimboo) from comment #25)
> Given the external dependencies we have for this patch I'm seeking for
> Andrew's feedback. This is an important feature we need for mozinstall to be
> able to install Firefox via the stub installer. So what is your take Andrew?
> Shall we move this as a general discussion about external deps to the tools
> newsgroup?

We can.. though I don't know that you'll get many action items out of it. I think checking the module into the tree is generally your best bet. This seems like it would be slightly more useful to have than the pefile one.

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This patch needs an update regarding the file paths. Mozbase is on mozilla-central now. Sorry, that this was so long in my queue. Somehow the request was totally missed. :( I will try to get to it soon.

Taking the bug from Jarek given that he is sadly not around anymore. A big thanks still goes to him for this wonderful work.

Comment 28

[Jarek Śmiejczak [:jotes]]

Hi, sorry but i was busy lately and totally out of time. I'll return at some point, but currently i can't make any declaration.

If you have any questions, feel free to ask - i'm following all of my old bugzilla bugs.