Open Bug 911048 Opened 6 years ago Updated 15 days ago

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

Categories

(Testing :: Mozbase, defect, P3)

All
Windows 7
defect

Tracking

(Not tracked)

People

(Reporter: whimboo, Unassigned)

References

Details

(Whiteboard: [qa-automation-wanted])

Attachments

(1 file, 6 obsolete files)

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.
Blocks: 910459
Whiteboard: [qa-automation-blocked]
If this is required only for windows we can try to use: http://code.google.com/p/pywinauto/
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.
OS: All → Windows 7
Well, I should at least CC or ask for information from Jeff.
Flags: needinfo?(jhammel)
(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.
Flags: needinfo?(jhammel)
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.
(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.
> 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.
Attachment #804920 - Flags: review?(hskupin)
Updated version with passing tests.
Attachment #804920 - Attachment is obsolete: true
Attachment #804920 - Flags: review?(hskupin)
Attachment #804937 - Flags: review?(hskupin)
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.
Attachment #804937 - Flags: review?(hskupin) → review-
> 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.
Patch updated (i've implemented previous comments).
Attachment #804937 - Attachment is obsolete: true
Attachment #807491 - Flags: review?(hskupin)
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?
Attachment #807491 - Flags: review?(hskupin) → review-
Assignee: nobody → jot
Status: NEW → ASSIGNED
> 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.
Attachment #807491 - Attachment is obsolete: true
Attachment #811671 - Flags: review?(hskupin)
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.
Attachment #811671 - Flags: review?(hskupin) → review-
> 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!
Attachment #811671 - Attachment is obsolete: true
Attachment #817501 - Flags: review?(hskupin)
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).
: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
(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 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?
Attachment #817501 - Flags: review?(hskupin) → review-
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.
Attachment #817501 - Attachment is obsolete: true
Attachment #823083 - Flags: review?(hskupin)
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.
Attachment #823083 - Flags: review?(hskupin) → review-
> 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.
I've added checking if user has rights of administrator. Please review again.
Attachment #823083 - Attachment is obsolete: true
Attachment #8365722 - Flags: review?(hskupin)
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?
Flags: needinfo?(ahalberstadt)
(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.
Flags: needinfo?(ahalberstadt)
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.
Assignee: jot → hskupin
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.
Whiteboard: [qa-automation-blocked] → [qa-automation-wanted]
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Priority: -- → P3
Attachment #8365722 - Flags: review?(hskupin)
You need to log in before you can comment on or make changes to this bug.