760377 - Let the installer return the installation folder and let consumers call get_binary() on their own

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

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

One thing we need for the uninstaller is the folder which contains the installed application. Right now we do a bit of magic guessing because we only have the binary path, which current gets returned by the installer. 

Instead the installer should return the installation folder and consumers should call get_binary() on their own to get the path of the binary. That way consumers can remember the installation path and pass it into the uninstaller. No magic guessing would be necessary anymore.

Be aware that this will change the API and we have to inform everyone who is making use of mozinstall. It should happen at least via the tools mailing list.

Steps to do:
1. Remove apps keyword argument from install() method
2. Make apps argument mandatory for get_binary() method so no magic guessing has to be done
3. Let the installer return the installation folder
4. Update the uninstaller to handle the installation folder instead of binary

Clint and Jeff, what do you both think about? IMHO such a change should happen before more test harnesses make use of mozinstall and a good time to make it a good API would be now.

Comment 1

[cmtalbert]

Sounds like a good and worthy API change to me.

Let's do it.

Comment 2

[Jeff Hammel]

(In reply to Clint Talbert ( :ctalbert ) from comment #1)
> Sounds like a good and worthy API change to me.
> 
> Let's do it.

wfm

Comment 3

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

Attachment: Patch

Pointer to Github pull-request

Comment 4

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

Comment on attachment 630732 [details]
Patch

This patch implements the above items we agreed on.

Comment 5

[Jeff Hammel]

Comment on attachment 630732 [details]
Patch

Looks good!

Comment 6

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

Pushed:
https://github.com/mozilla/mozbase/commit/ab72045d5e75ac76502f93d30f1547de6eac8d94