(qbittorrent) Updated to embed installer #442

AdmiringWorm · 2016-12-03T17:52:20Z

qBittorrent uses GPL license which allows redistribution

I'm unsure about the creation of embedded packages, so any feedback would be appreciated.

gep13 · 2016-12-03T17:58:44Z

Have a look at the ChocolateyGUI package. It is quite old, and needs updating, but the premise of embedding the msi within the package is shown in there.

gep13 · 2016-12-03T17:59:17Z

Heading out for the night, but will check in tomorrow to see if there are any questions.

AdmiringWorm · 2016-12-03T18:05:47Z

gotcha.
From looking at the chocolateyGUI package, this should work then (with that I mean it should be accepted).
I was mostly curious if there was something additional I needed to include, the file is downloaded by au
in the BeforeUpdate function and replaces the path/filename specified as arguments to Install-ChocolateyInstallPackage.

gep13 · 2016-12-03T18:09:15Z

Nope, don't think so. As long as the nuspec files section isn't explicitly including files, then the downloaded file should get picked up and included in the package that is generated by AU. If we are going down this route, we will need to include a verification.txt to allow moderators to verify that the included file is legitimate.

AdmiringWorm · 2016-12-03T18:10:39Z

If we are going down this route, we will need to include a verification.txt to allow moderators to verify that the included file is legitimate.

Like where they can download the file and verify the checksum?

gep13 · 2016-12-03T18:12:41Z

Yip. Check the output of Choco new to see what is expected. It is mainly used by moderators, but can be used by end users as well to additionally verify the included installer.

majkinetor · 2016-12-03T18:22:15Z

automatic/qbittorrent/update.ps1

@@ -1,28 +1,36 @@
 import-module au
 import-module "./../../extensions/extensions.psm1"


should be removed

will do, forgot about that one.

majkinetor · 2016-12-03T18:23:49Z

automatic/qbittorrent/update.ps1

+  $download_page = Invoke-WebRequest -uri $Latest.URL32 -UseBasicParsing
+  $url = $download_page.links | ? class -eq "direct-download" | select -expand href
+
+  $client = New-Object System.Net.WebClient


Why not using iwr for this too ?

because iwr is notoriously slow for downloading files, so to save time if there is multiple packages
needing to download files, I'm using .net instead.

majkinetor · 2016-12-03T18:23:57Z

LGTM

Like where they can download the file and verify the checksum?

You can just put $Latest table or parts of it in verification.txt

Some questions:

Do we keep the setup in the tools dir forever or not ?
Will the tools dir build up old installers ?
Windows installer dir is already cacheing it too. Could we delete it after installation to save space ?

gep13 · 2016-12-03T18:28:44Z

@majkinetor said...
You can just put $Latest table or parts of it in verification.txt

Not sure what you mean by this. Can you elaborate?

The idea of the verification.txt is that it is a list of human followable steps to verify that the bundled installer, and associates checksum are correct. I.e. Go to this website, click on download link, run this command to generate the hash, verify against chocolateyInstall.ps1.

AdmiringWorm · 2016-12-03T18:40:08Z

@majkinetor said
Do we keep the setup in the tools dir forever or not ?

I don't see any reason to do that, and since the .exe file is already is already on the ignore list for the repo it shouldn't be commited (unless AU overrides it though).

@majkinetor said
Will the tools dir build up old installers ?

Good question, I don't really know. definitely something that needs to be tested though.

@gep13 said
verify against chocolateyInstall.ps1.

I'm guessing you meant verifying against the installer/archive, right?

gep13 · 2016-12-03T18:43:16Z

@AdmiringWorm said...
guessing you meant verifying against the installer/archive, right?

Well yes, both. I.e. Verify against the checksum that is contained within the installation script as well.

AdmiringWorm · 2016-12-03T18:46:11Z

@gep13 It would be enough with something like this right?

The installer have been downloaded from the alternative sourceforge
mirror listed on <http://www.qbittorrent.org/download.php>
and can be verified like this:

1. Go to <download location> and download the installer
2. Use powershell function 'Get-FileHash' or the chocolatey utility 'checksum.exe'
   to verify the checksum matches 'checksum hash'

the checksum in the installation script? isn't the checksum only in the installation script if
we download it from the during installation, not when it has been embedded.
Or am I misunderstanding something perhaps?

gep13 · 2016-12-03T18:48:24Z

@AdmiringWorm ah, sorry, wasn't thinking there.

Yes, you are right. The expected checksum would go into the verification.txt file, and wouldn't be in the install script.

My bad...

ferventcoder · 2016-12-03T19:06:36Z

automatic/qbittorrent/tools/VERIFICATION.txt

+1. Go to <http://sourceforge.net/projects/qbittorrent/files/qbittorrent-win32/qbittorrent-3.3.7/qbittorrent_3.3.7_setup.exe/download>
+   and download the installer
+2. Use powershell function 'Get-FileHash' or the chocolatey utility 'checksum.exe'
+   to verify that the installer matches the sha256 checksum '49AE9A0ADFC3272BEC38822C528F732D9495B79A2A7CA934F8C6635237B15D07'


BEAUTIFUL! Thanks for catching this as a necessity

no problem, is there anything else should be in the verification.txt file?
or any additional steps I need to include?

ferventcoder · 2016-12-03T19:08:55Z

You will also need a LICENSE.txt file. The reason we ask for this is protection of maintainers and the community. A remote licenseUrl cannot be controlled and could be changed. You can see how this could lead to problems down the road if someone changes the license and then claims that the license did not allow for distributions.

For protection, also place the LICENSE.txt file in the package with the embedded software.

AdmiringWorm · 2016-12-03T19:09:27Z

@ferventcoder alright, will do

ferventcoder · 2016-12-03T19:11:57Z

If we are going down this route, we will need to include a verification.txt to allow moderators to verify that the included file is legitimate.

This is not just for moderators. This is also so someone looking can take the checksum for the binary (listed on the package page under files) and use it to verify the remote location is exactly the same.

FWIW - when you push a package, during the gathering of all included files, if the file is a binary, the Gallery grabs the checksum values for md5, sha1, sha256, and sha512 to ease verifying it with the remote source.

I'd still add the checksum value to the verification.txt file as well, that adds additional value IMHO.

ferventcoder · 2016-12-03T19:13:17Z

I wish github would publish checksums when folks put up binaries under releases. That would be most helpful.

AdmiringWorm · 2016-12-03T19:22:47Z

@ferventcoder said
I wish github would publish checksums when folks put up binaries under releases. That would be most helpful.

Same here, I actually find it a little odd that they doesn't

majkinetor · 2016-12-03T19:48:39Z

Do we keep the setup in the tools dir forever or not ?

I didn't mean about repo, but about installation. Installer can delete the setup after installation for example to save space, otherwise, every one will be x3 (tools, TEMP, windows cache).

automatic/qbittorrent/tools/VERIFICATION.txt

We could have a template for this, could be part of the package directory and AU can modify it during update. Something like:

+1. Go to $releases page and download the installer
+2. Use powershell function 'Get-FileHash' or the chocolatey utility 'checksum.exe'
+   to verify that the installer matches the sha256 checksum is $Latest.Chekcusm

Then this could be used as such for majority of packages.

majkinetor · 2016-12-03T19:50:57Z

About keeping the binaries in repository I wouldn't do that for start, but that is probably the best solution. But really, if Choco ever goes out of buisnis I guess there will be migration plan and it will certainly not be shut down over night so we can make crawlers to downlolad older packages that we need.

And honestly, in huge majority of cases you don't really need particular version but latest one so older versions are most of the time IMO irrelelvant (i mean, its one thing to have version for library and who needs specific version of lets say qbittorent).

majkinetor · 2016-12-03T19:55:38Z

You will have to take into account that tools folder should be cleaned before each new install. On AV its already empty, but local runs may keep outdated files.

AdmiringWorm · 2016-12-03T19:55:51Z

Sorry, my mind went straight to the repo not on the users machine.

regarding saving space, it wouldn't be stored in TEMP though but it will be stored in tools directory and possibly in windows cache (although my windows\installer directory only contains msi files, not .exe files).

We could have a template for this, could be part of the package directory and AU can modify it during update.

That would definitely be useful.

About keeping the binaries in repository I wouldn't do that for start, but that is probably the best solution.

There is no need to store them in the repository, with this PR I've added a .gitignore file in the automatic directory to ignore common binary files.
They are packed inside the nupkg package, so there is just no need to store them anywhere else. IMHO

AdmiringWorm · 2016-12-03T19:57:02Z

You will have to take into account that tools folder should be cleaned before eacn new install. On AV its already empty, but local runs may keep outdated files.

That may be true, I haven't really tested updating yet, but I will.

majkinetor · 2016-12-03T19:58:27Z

regarding saving space, it wouldn't be stored in TEMP though

Right, forgot you are using different choco installer.

They are packed inside the nupkg package, so there is just no need to store them anywhere else. IMHO

I know, the point was that then we depend on chocolatey.org existence. If chocolatey.org is out one day packages on this repo become unusable.

majkinetor · 2016-12-03T19:59:18Z

But even with no changes its certainly MUCH better then what we have now where exes dissapear from vendor site.

majkinetor · 2016-12-03T20:19:45Z

I think chocolateyUninstall.ps1 should be modernized.

AdmiringWorm · 2016-12-03T20:21:24Z

I think chocolateyUninstall.ps1 should be modernized.

Using the uninstall helper in chocolatey-core extension instead?

ferventcoder · 2016-12-03T20:45:33Z

We could explore git-lfs for binaries.

ferventcoder · 2016-12-03T20:45:58Z

Then you get best of both worlds here

AdmiringWorm · 2016-12-03T20:55:11Z

We could explore git-lfs for binaries.

That's definitely a possibility, can't say I've ever tried git-lfs before though so I don't know how it is used (YET).
I also think lfs storage is free for the first GB, but not entirely sure about that though.

AdmiringWorm · 2016-12-04T13:12:02Z

I think it is done now, only thing to find out first it whether we should store the binaries in the repo or not.
If we are I need to remove the additional .gitignore file I've added.

EDIT:
If this is accepted, I'll squash all commits before merging it.

majkinetor · 2016-12-04T17:50:45Z

I think it is done now, only thing to find out first it whether we should store the binaries in the repo or not.

Not for now. I just tried git-lfs and its really epic but we need to either use Github enterprise for chocolatey user or mount lfs server on existing chocolatey infrastructure. Without deduplication size will not be trivial - 15 versions of smplayer alone for example will take ~1GB. I think that 1 TB of space without deduplication should be enough for a couple of years for all packages that could ever become embeded. With dedup, I beleive this could be order of magnitude lower.

Other then that I think git-lfs is epic as given the chocolatey client, repo becomes another form of decentralized gallery that is easy to utilize independent of the chocolatey.org and it solves the 404 problem completelly.

majkinetor · 2016-12-04T17:54:48Z

So far I see just one downside of embeding - you will typically download x2 of what you need (as package contains both x32 and x64 and non-embeded packages download just one). This is not ideal for big tools but again those should probably not be embeded. Alternative is to use separate x32 and x64 package (like we already have for some packages) which is something I dont think we should generally practice.

majkinetor · 2016-12-04T18:39:16Z

I think it is done now

Qbittorrent does not register itself. Perhaps you could add at the end:

$packageName = $packageArgs.packageName
$installLocation = Get-AppInstallLocation $packageName
if ($installLocation)  {
   Write-Host "$packageName installed to '$installLocation'"
   Register-Application "$installLocation\$packageName.exe" qbit
   Write-Host "$packageName registered as qbit"
}
else { Write-Warning "Can't find $PackageName install location" }

The details are on chocolatey/choco#1072. This is quite practical for testing too as it allows you to very quickly launch it.

+              $installLocation = Get-AppInstallLocation $packageArgs.softwareName
+              if ($installLocation) {
+                Write-Host "$($packageArgs.packageName) installed to '$installLocation'"
+                Register-Application "$installLocation\qbittorrent.exe"

		@@ -1,28 +1,36 @@
		import-module au
		import-module "./../../extensions/extensions.psm1"

(qbittorrent) Updated to embed installer #442

(qbittorrent) Updated to embed installer #442

Conversation

AdmiringWorm commented Dec 3, 2016

gep13 commented Dec 3, 2016

gep13 commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

gep13 commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

gep13 commented Dec 3, 2016

majkinetor Dec 3, 2016

Choose a reason for hiding this comment

AdmiringWorm Dec 3, 2016

Choose a reason for hiding this comment

majkinetor Dec 3, 2016

Choose a reason for hiding this comment

AdmiringWorm Dec 3, 2016

Choose a reason for hiding this comment

majkinetor commented Dec 3, 2016

gep13 commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

gep13 commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

gep13 commented Dec 3, 2016

ferventcoder Dec 3, 2016

Choose a reason for hiding this comment

AdmiringWorm Dec 3, 2016

Choose a reason for hiding this comment

ferventcoder commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

ferventcoder commented Dec 3, 2016 • edited

ferventcoder commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

majkinetor commented Dec 3, 2016

majkinetor commented Dec 3, 2016

majkinetor commented Dec 3, 2016 • edited

AdmiringWorm commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

majkinetor commented Dec 3, 2016

majkinetor commented Dec 3, 2016 • edited

majkinetor commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

ferventcoder commented Dec 3, 2016

ferventcoder commented Dec 3, 2016

AdmiringWorm commented Dec 3, 2016

AdmiringWorm commented Dec 4, 2016 • edited

majkinetor commented Dec 4, 2016

majkinetor commented Dec 4, 2016

majkinetor commented Dec 4, 2016 • edited

AdmiringWorm commented Dec 5, 2016

majkinetor Dec 5, 2016 • edited

Choose a reason for hiding this comment

majkinetor commented Dec 5, 2016

ferventcoder commented Dec 3, 2016 •

edited

majkinetor commented Dec 3, 2016 •

edited

majkinetor commented Dec 3, 2016 •

edited

AdmiringWorm commented Dec 4, 2016 •

edited

majkinetor commented Dec 4, 2016 •

edited

majkinetor Dec 5, 2016 •

edited