Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement package changes as recommended by @jberezanski #20

Merged
merged 1 commit into from
Sep 28, 2015
Merged

Implement package changes as recommended by @jberezanski #20

merged 1 commit into from
Sep 28, 2015

Conversation

acaire
Copy link
Contributor

@acaire acaire commented Sep 22, 2015

Added in recommended changes from @jberezanski apart from:

The detection logic is not forward compatible. Once there is a future .NET 4.x release (for example, .NET 4.7), the package will not detect it and will proceed to install 4.6, even though a later release is already installed. Please fix the condition to use greater-than-or-equal, i.e. "... -and ($test -ge 393295)".

I'm not sure we should be blocking an installation if a newer major release is installed, considering the approved package name AFAICT will be dotnet4.6, thus explicitly requesting that version.

A reboot might be required before the upgraded .NET is actually visible. Perhaps the user should be warned about that? Otherwise, it might give the impression that the package does not work.

This would be great functionality to have, however none of the previous releases did this and IMHO this is better implemented in Chocolatey rather than bloating the ps1 by having to do something convoluted like this

@jberezanski
Copy link

I'm not sure we should be blocking an installation if a newer major release is installed

All .NET releases in the 4.x family after the original 4.0 have been in-place upgrades. If, for example, 4.6 is already installed, it makes no sense to try to install 4.5.2. It only unnecessarily makes the user download the installer and wait for it to execute, without making any changes to the system.

IMHO this is better implemented in Chocolatey rather than bloating the ps1 by having to do something convoluted

I agree that ultimately Chocolatey should handle it (with necessary information provided by packages, e.g. which exit code signals the need to reboot). Here I was thinking about a simple, unconditional Write-Host informing the user that a reboot might be required.

@acaire
Copy link
Contributor Author

acaire commented Sep 24, 2015

All .NET releases in the 4.x family after the original 4.0 have been in-place upgrades. If, for example, 4.6 is already installed, it makes no sense to try to install 4.5.2. It only unnecessarily makes the user download the installer and wait for it to execute, without making any changes to the system.

I see your logic, except why would a user be instructing a package manager to install an implied version of the software that they do not want (or an older version than one they already have)? I can only think of two reasons: to downgrade, or install two versions side-by-side.
I'm not familiar with .NET packaging, but in your review you remarked that an older version would be installed, but now you've stated that executing an older installer would do nothing - Do they allow side by side installations? If that's not the case it seems like it might be better to move to a package simply named 'DotNet' or 'DotNet4X', and start using Version params instead.

I agree that ultimately Chocolatey should handle it (with necessary information provided by packages, e.g. which exit code signals the need to reboot). Here I was thinking about a simple, unconditional Write-Host informing the user that a reboot might be required.

That's fair - I would've thought that this would already be something a user with an existing version of .NET blocking a 'reboot-free' installation would be across, but we may as well put something in to that effect.

@jberezanski
Copy link

why would a user be instructing a package manager to install an implied version of the software that they do not want (or an older version than one they already have)?

As a dependency of another package, such as this one or that one.

I'm not familiar with .NET packaging, but in your review you remarked that an older version would be installed, but now you've stated that executing an older installer would do nothing - Do they allow side by side installations?

Perhaps I was too succint in the review. I meant that the package (i.e. your chocolateyInstall.ps1 script) would attempt to install the older version, i.e. download and execute the installer. The outcome of that attempt would depend on the logic implemented in the installer. In case of .NET 4.x, the installers do nothing if a later .NET 4.x release is already installed.

The following three .NET families can be installed side-by-side:

  • 1.x (1.0, 1.1)
  • 2.x-3.x (2.0, 3.0, 3.5, 3.5.1)
  • 4.x (4.0, 4.0.1, 4.0.2, 4.0.3, 4.5, 4.5.1, 4.5.2, 4.6)

Within each family, newer releases are in-place upgrades.

If that's not the case it seems like it might be better to move to a package simply named 'DotNet' or 'DotNet4X', and start using Version params instead.

Yes, I believe that would be a better packaging strategy - one package per family. However, since we already have separate packages for earlier releases, it might take considerable effort to reorganize it now, in particular to convince the maintainers of those packages. (Incidentally, I've begun a similar attempt for Visual Studio Update packages.)

One more argument: the official .NET deployment guide specifically requests checking for greater or equal Release value ("Detecting the .NET Framework" section).

That's fair - I would've thought that this would already be something a user with an existing version of .NET blocking a 'reboot-free' installation would be across, but we may as well put something in to that effect.

To provide some context: my request was motivated by this Disqus comment for the dotnet4.5.2 package, where the user initially thought the package did not install correctly.

@jberezanski
Copy link

One more thing. I just noticed the package downloads the web installer, not the full/offline one. It would be preferable to use the full one, as it would give Chocolatey complete control over the download process, enabling features such as url redirection (for corporate scenarios) or caching (for offline installation). Thoughts?

@acaire
Copy link
Contributor Author

acaire commented Sep 25, 2015

@jberezanski ahh I see, thanks for the clarification and extra info 😃

Pushed a new commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants