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

Support notification on `pwsh` startup when a new update is available #162

Open
wants to merge 4 commits into
base: master
from

Conversation

@daxian-dbw
Copy link
Member

commented Mar 24, 2019

Support notification on pwsh startup when a new update is available.

Today, to find out whether a new version of PowerShell is available, one has to check the release page of the PowerShell\PowerShell repository, or depend on communication channels like twitter or GitHub Notifications. It would be convenient if pwsh itself can notify the user of a new update on startup.

SteveL-MSFT and others added 3 commits Mar 25, 2019
Fix typo -- Non-Goals
Co-Authored-By: daxian-dbw <dongbow@microsoft.com>
Fix typo -- per day
Co-Authored-By: daxian-dbw <dongbow@microsoft.com>

#### How to synchronize update checks

The most challenging part is to properly synchronize the update checks started from different `pwsh` processes,

This comment has been minimized.

Copy link
@rjmholt

rjmholt Apr 11, 2019

Member

Just to play devil's advocate, would it be possible to use a global Mutex for this instead of the initial sentinel file? Or is that not desired even if possible?

This comment has been minimized.

Copy link
@daxian-dbw

daxian-dbw Apr 11, 2019

Author Member

Mutex is certainly possible, but it will result in far more issues, mostly deadlock due to the abandoned mutex. We used mutex for the module analysis cache file back before PSv5, and there were many deadlock issues reported due to the mutex. We changed to use a simple file lock in PSv5.

This comment has been minimized.

Copy link
@daxian-dbw

daxian-dbw Apr 12, 2019

Author Member

Also, mutex is a model of "block till I grab the lock" while what I need here is "give up if I'm unable to grab the lock".

@chuanjiao10

This comment has been minimized.

Copy link

commented Jun 4, 2019

I recommend:
We should build a boolean variable on the disk.
Upgrade and keep the latest version (default) or not upgrade to keep the existing version.

You can use the registry to store this variable on win. Where is it stored on linux?

According to the value of this variable, we make automatic background upgrade function, especially the win version of powershell7. Instead of a new update is available.

Users are lazy, if only notifications, then powershell-team is bound to face a lot of powershell version fragmentation!

So, notifications "if you do not change the variable,it will automatic background upgrade every 3---6 month" better.

such as when `pwsh` is in a container image.
Hence, you should be able to suppress them altogether by setting an environment variable.

### Non-Goals

This comment has been minimized.

Copy link
@SteveL-MSFT

SteveL-MSFT Jun 6, 2019

Member

Should add that auto-updates is a non-goal


7. The notification and update check are not needed in some scenarios,
such as when `pwsh` is in a container image.
Hence, you should be able to suppress them altogether by setting an environment variable.

This comment has been minimized.

Copy link
@iSazonov

iSazonov Jun 6, 2019

Contributor

For Enterprise we need add the suppress option in Policy settings.

During the startup, `pwsh` creates a `Task` of the update check work,
but delays the task run for 3 seconds by using `Task.Delay(3000)`.
The typical startup time for `pwsh` with a moderate size profile should be less than 1 second.
Given that, I guess it's reasonable to delay the update check work for 3 seconds,

This comment has been minimized.

Copy link
@iSazonov

iSazonov Jun 6, 2019

Contributor
Suggested change
Given that, I guess it's reasonable to delay the update check work for 3 seconds,
Given that, it's reasonable to delay the update check work for 3 seconds,
and the update check related files for that version of `pwsh` are put there exclusively.
In this way, the update information for different versions of `pwsh` doesn't interfere with each other.

- Windows: `$env:LOCALAPPDATA\Microsoft\PowerShell\6.2.0`

This comment has been minimized.

Copy link
@iSazonov

iSazonov Jun 6, 2019

Contributor

Why do we need the folder? We could place all information in the file name based on GitCommitId.
Then we could compare the file GitCommitId and current PowerShell process GitCommitId. Also we could update LastWriteTimestamp (if we done success internet check) instead of creating todayDoneFileName file.
If update task will need to do internet check it will try to lock the file. If success it do the check. Otherwise skip (another process already do the check.)

It would be much better if we can have the latest release/pre-release information stored in a well-known URL,
to make the query easier and take less time.
- GitHub API doesn't support querying for the latest pre-release,
so we need to hit the 'get-all-releases' API `https://api.github.com/repos/PowerShell/PowerShell/releases`.

This comment has been minimized.

Copy link
@iSazonov

iSazonov Jun 6, 2019

Contributor

We could query our custom tools/metadata.json file at first step. It is more simple and fast.
This allow us to trigger new versions in moderated way (after we check that new packages was published and work well).

and thus the deserialization is expensive, taking about 650 ms on my dev machine.
We only care about the `tag_name` and `published_at` attributes,
so it would be desirable to optimize the deserialization to skip the unneeded.
- If there is a new update, create the file `_update_<version>_<publish-date>` if one doesn't exists yet;

This comment has been minimized.

Copy link
@iSazonov

iSazonov Jun 6, 2019

Contributor

As I mentioned above I think we could pack all information in name of one file.

@adityapatwardhan

This comment has been minimized.

Copy link
Member

commented Jul 16, 2019

@daxian-dbw Should we have a different message mentioning a security update is available for GA releases? The urgency of updating a preview v/s a GA with security patch is different.

@daxian-dbw

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2019

@adityapatwardhan Is there a way to know a release is a security release by querying GitHub/release? Or do we need to have a back end to provide that information?

@joeyaiello joeyaiello added this to the 7.0-Consider milestone Aug 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.