KiCad / kicad-winbuilder

Windows builder for the KiCad project based on the MSYS2 MinGW system [moved to https://gitlab.com/kicad]
51 stars 24 forks source link

Add lightweight installer that downloads 3d models #86

Open qu1ck opened 5 years ago

qu1ck commented 5 years ago

Current installer is still generated with -full designation. New, light installer downloads 3d models from github only if requested. This should save about 800mb on each installer download size. This is especially useful for nightlies but people who use stable but with their own libs or git checkout will appreciate this too.

Selection tree

img

Downloading progress

img

Prompt for when download fails:

img

nickoe commented 5 years ago

Thank you for your proposal. I am not sure if I like the concept here. I think it would be nicer that one does not need to wait fpr a download when one have already downloaded the installer.

I would think I would rather prefer an option to the copydlls.sh script to include or skip the 3d models. IIRC the nsis script already do update the checkboxes properly if the 3d files does not exist.

qu1ck commented 5 years ago

This will generate 2 separate installers. One full installer which is same as current one, with everything included. And a light one which will download on demand but by default skips 3d models. This gives even more control to users than what you are proposing.

nickoe commented 5 years ago

I have a hard time to see why it is better to have a slightly smaller installer that spends time downloading a zip and then extracting it. I don't think the install process will be faster this way.

I think it is better to keep it simple, so essentially what I tried to describe in https://github.com/KiCad/kicad-winbuilder/issues/53#issuecomment-472048853 Are you willing to attempt that?

The patch looks good in general, there are some minor style things I would change, but as I am not sure we should go with this solution I will not make inline review right now.

qu1ck commented 5 years ago

Here is my thinking:

Users that want everything will download full installer. Users that want small installer will use smaller one. However if they change their mind and want to install 3d models anyway, they can, even with small installer. But unless they explicitly choose that option, this installer will not download anything and won't need internet connection, same as big one.

I think this is pretty much in line with the comment you linked, except there is additional option to download models with light installer. This is what I meant by giving users more control.

Also it's not a slightly smaller installer, it's 1.1gb vs ~150mb

If you are strictly opposed to do any downloading in the installer I could simplify this and just omit that whole section in light installer. But what I have already has no downsides that I can see and I already did the work...

sethhillbrand commented 5 years ago

I like this approach. This feels like a standard installer for larger programs. I might make all data downloaded instead of just the 3d models but that is a nitpick. Overall, this feels like a large improvement, especially for folks on metered connections.

EeliK commented 5 years ago

For the nightly builds it's certainly best to have one small installer and download options inside it. Splitting the installer to two (1 for program, 1 for libraries) would be more work for the users. One installer is easy: for the first time users you download the installer, choose library downloading and install. For repeated use you just left library options unchecked.

On the other hand a separate installer for the libraries wouldn't be bad, either. It would be used only once or rarely.

In my opinion all the libraries - downloadable or inside the installer - could be unchecked by default. That's probably the most used option for the nightly build users. Other options should also be unchecked by default. This could be asked in the user forum - what are the usual options which nightly build users use. Shall I?

BTW, it looks like (in the commit code) that there's no library versioning, it just downloads the latest master zip. Is it a potential problem that for example a new file format feature is taken into the libraries but the installer is a bit older, so that the program installed by the installer can't handle the libraries? That would be a reason to include the symbol and fp libraries in the installer itself. After all they don't use so much space. 3D model libraries, on the other hand, are independent from KiCad's formats and shouldn't cause problems, while they are responsible for most of the size.

qu1ck commented 5 years ago

I only made 3d models downloadable because they are by far the largest. Not packaging 3d models saves 800mb while not packaging other libs only saves additional 15mb. I think it's just not worth it.

EeliK, there is versioning, script detects github tag from the program version string. It only defaults to master if version is not of form N.N.N-something which is the case for nightlies.

qu1ck commented 5 years ago

Built both installers for current master: kicad-r13796.6cd7d9fb8-x86_64.exe 133mb kicad-r13796.6cd7d9fb8-x86_64-full.exe 921mb

sethhillbrand commented 5 years ago

Ah. That makes sense then.

qu1ck commented 5 years ago

So what are next steps to get this merged? @nickoe do you still have objections? It would be cool if 5.1.3 could be generated with new installer.

qu1ck commented 5 years ago

Nick, Please allow me to clarify where our differences are.

You would like to have 2 installers that are:

Here is what this change implements:

Is my understanding correct? If so I don't see how my proposal is not a strictly superset of your proposal and therefore no reason to not adopt it. Please let me know if I'm mistaken.

In general modern installers can be categorized as:

I believe KiCad is in the third category and I chose to draw the line between packaged and "extras" at 3d models because of their size and easily available download url provided by github.

The reason why I'm reluctant to just do what you asked is that compiling and testing installers takes many hours. Single build with winbuilder takes about 1h30m on my machine. If we can agree on the end goal beforehand it will save me a lot of time in testing and my current proposal is already fully tested.

qu1ck commented 5 years ago

@nickoe hope you had good vacation :) Going back to this, I asked about the change on the mailing list and it received unanimous support, including Wayne's (https://lists.launchpad.net/kicad-developers/msg41681.html), except for not rushing it for 5.1.3 and I agree with that. Can we test this on nightlies?

Also I addressed your other comments and split some commits, let me know if you want me to send separate PR to merge those first.

qu1ck commented 5 years ago

Friendly ping.

EeliK commented 5 years ago

I agree with the ping and have thought about it myself many times. Especially when I had to use a 3G modem and downloading ~1GB took half an hour. As someone once commented in the user forum, the need for downloading and installing a new nightly build often comes and goes quickly. When downloading is over you are already doing other things.

Plus, whether Trump believes it or not, climate change may be real, and people have made calculations about how much power it takes to move bits over internet... it doesn't come for free even when I don't have to pay for extra bits.

sethhillbrand commented 4 years ago

@nickoe Were there remaining outstanding issues with the PR?

nickoe commented 4 years ago

As requested in https://github.com/KiCad/kicad-winbuilder/pull/86#pullrequestreview-264166742

I would rather we split the logic to enable lite and full installer to a separate pull request before adding the download stuff. I am still not sure if I like the download stuff, but lets just move on with updating the script to make a lite and full installer. This should fulfill #53 (comment)

So in short, I would rather we update the script to be parameterized to build a lite build.

sethhillbrand commented 4 years ago

Just to make sure I understand your comment: you are saying that you will not merge this until the download option is removed?

qu1ck commented 4 years ago

@nickoe can you answer my question from https://github.com/KiCad/kicad-winbuilder/pull/86#issuecomment-513405121 ? Why exactly are you against download stuff?

EeliK commented 4 years ago

"Why exactly are you against download stuff?"

I wonder that, too. After all downloading is just an option in that installer. It wouldn't automatically download anything, so it's as good as two non-downloading (with and without 3D models) together. The original one-bunch installer would be needed only for complete offline installation procedure, and that's hardly needed for nightly builds.

nickoe commented 4 years ago

@qu1ck I think the solution you proposed do seem to work alright, but I also think it is problematic by design.

We need to settle on the way forward, not just throw a rock at it and hope for the best. Conceptually I think it is not great that we separate the 3D models away from the symbols, footprints and templates in this way. I consider the library one unified thing by itself, and the 3d models could be an additional option for the library install.

I think we need to have the lite installer only bundle the stuff from the kicad source and the kicad-i18n repo.

Thank you for rebasing. I can merge the first two commits and maybe wait with the download feature of the installer.

I think you are a valuable contributor to the KiCad community, but I don't want to rush these kind of things, it could easily lead to user confusion, especially if you are a user not actively involved in the development. I admit I have let this PR rot a bit, partly, because it does not seem like a finished solution to me as is.

EeliK commented 4 years ago

Do you mean you would accept a lite installer which doesn't include footprints, symbols, 3D models etc. but has download options for them? If so, Seth said about it:

" I might make all data downloaded instead of just the 3d models but that is a nitpick."

And qu1ck said:

"Not packaging 3d models saves 800mb while not packaging other libs only saves additional 15mb. I think it's just not worth it."

But I believe qu1ck is happy to make the changes to make all data downloadable (although he must talk for himself).

I agree it would be a cleaner solution. As far as I can see nobody would complain about that (compared to downloable 3D library only). Personally I always untick all options from the nighlty installer except when I deliberately want to update libraries which is almost never.

Whether it causes confusion depends on other things than plain existence of full and lite installers. For example, the lite installers can be put to different directories in the server. Those who know about them can use them. The word would reach most of the active testers, I think, and they are those who need them. Average users of the stable versions don't need them.

Anyways, I'm happy with an installer which doesn't have libraries or other data at all. As I said, I rarely need that data when upgrading to the latest nightly build, and it would always be available in the full installer. It's also probably easier to explain to users that "there are two installers, full with libraries and lite without".

nickoe commented 4 years ago

Do you mean you would accept a lite installer which doesn't include footprints, symbols, 3D models etc. but has download options for them?

Yes.

It's also probably easier to explain to users that "there are two installers, full with libraries and lite without".

Exactly.

qu1ck commented 4 years ago

No problem, I will update this patch to make all libraries downloadable. I can totally understand your reasoning.

qu1ck commented 4 years ago

@nickoe now all 3 libs are downloadable options in lite installer, disabled by default.

nickoe commented 4 years ago

@qu1ck Would it be possible to refactor DownloadAndExtract to be able to download all zips and then extract such that we are only asked once to delete or keep the downloaded files?

qu1ck commented 4 years ago

@nickoe done. I also updated the lang strings to reflect that all libs are downloaded in small installer.

nickoe commented 4 years ago

@qu1ck It looks like it deletes files before installing the downloaded libraries. I don't think we should do this. I think we should just overwrite. If the use does not want old files left he should uninstall before installing instead. The rest of the installer just overwrites anyways.

qu1ck commented 4 years ago

@nickoe done. Now files are copied over instead of delete and replace.

sethhillbrand commented 4 years ago

@nickoe It looks like your requested changes have been implemented. Now that 5.1.5 is out, are there any obstacles to merging this PR?