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

Draft of List, Upgrade and Uninstall. #397

Open
wants to merge 13 commits into
base: master
from

Conversation

@KevinLaMS
Copy link
Contributor

@KevinLaMS KevinLaMS commented Jun 4, 2020

This includes issue 120, 121 and 119.

Microsoft Reviewers: Open in CodeFlow
KevinLaMS added 2 commits May 29, 2020
@KevinLaMS KevinLaMS requested a review from microsoft/windows-package-manager-admins as a code owner Jun 4, 2020
Apps that are free, will be downloaded directly when requested.

***Pay apps [NOT SUPPORTED]***
Apps that are for purchase, will not be allowed to be submitted. If an app from the store requires commerce, to run, we will not allow them in the Windows Package Manager repository.

This comment has been minimized.

@megamorf

megamorf Jun 4, 2020

What about paid apps that have been purchased already?

This comment has been minimized.

@yao-msft

yao-msft Jun 4, 2020
Contributor

If the entitlement is already granted, I think we can continue with installation.

@megamorf
Copy link

@megamorf megamorf commented Jun 4, 2020

@KevinLaMS Some general feedback.

Based on the contents on these drafts you haven't worked with Markdown that much so here are some potential issues that can be solved fairly quickly:

  • When using inline or fenced code formatting you don't need to escape characters (e.g. angle brackets) with a backslash `winget list [[-q] <query>] [<options>]`
  • Instead of using HTML tags for formatting try to stick with the markdown versions, e.g. **bold string** instead of <b>bold string</b>
  • All path references in markdown use a regular slash as path separator
  • lists, headings and tables should be surrounded by new lines

See: https://www.markdownguide.org/basic-syntax/

I like where this is going and will add more feedback later. 👍

Copy link

@fmeyertoens fmeyertoens left a comment

Just a few typos.

doc/specs/#117-Windows-Store-Support.md Outdated Show resolved Hide resolved
doc/specs/#117-Windows-Store-Support.md Outdated Show resolved Hide resolved
doc/specs/#120,121,119-Update-Uninstall-List.md Outdated Show resolved Hide resolved
doc/specs/#120,121,119-Update-Uninstall-List.md Outdated Show resolved Hide resolved
doc/specs/#120,121,119-Update-Uninstall-List.md Outdated Show resolved Hide resolved
doc/specs/#120,121,119-Update-Uninstall-List.md Outdated Show resolved Hide resolved
Copy link

@martinmine martinmine left a comment

Some comments from the discussion in #120


## Abstract

For our preview of the Windows Package Manager, the goal was to enable install of apps. In order to be a package manager, the Windows Package Manager must be able to list all installed packages, communicate if there is an update ane uninstall apps.

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

Small typo:

Suggested change
For our preview of the Windows Package Manager, the goal was to enable install of apps. In order to be a package manager, the Windows Package Manager must be able to list all installed packages, communicate if there is an update ane uninstall apps.
For our preview of the Windows Package Manager, the goal was to enable install of apps. In order to be a package manager, the Windows Package Manager must be able to list all installed packages, communicate if there is an update and uninstall apps.
| **-e,--exact** | Find the application using exact match. |
*Upgrade app when no update is available*

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

Another aspect to consider: What would happen if the user updates an app that is not installed? Should it then display an error or install said app?

*Upgrade app when no update is available*
If the user attempts to update an app that there is no known update for, the package manager will issue an error.

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

Should winget exit with a non-successful status code if this is the case?

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

Yes, I believe so. If the command requested cannot be executed, the process should return a failure exit code.

This comment has been minimized.

@rmarquis

rmarquis Jun 5, 2020

This seems counter intuitive to me, and it's also not what Linux package managers do as far as I know (using pacman here). If the purpose of the upgrade command is to check if newer version of package(s) are available and get them, then it didn't fail if the last available package(s) are installed already. The output should be different (f.e. "there is nothing to do") but the command would be considered a success anyway.

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 5, 2020
Member

I guess it comes down to what the purpose is deemed to be semantically. I am not opposed to the purpose being "Make sure I have the latest version of this", which then I agree with you that no error is correct. I was reading it more as "Get a newer version of this", which while I agree is a subtle difference, it is still the difference between a command that cannot be executed, versus one that can.

I suppose there is the question of what is done in the --all case. In that case, should there be an error if no updates are available for anything? That feels wrong to me, as I don't think anyone would ever say "Get a newer version of everything". So for consistency, I believe that the semantics should be Ensure, not Get.

So I agree that there should not be an error exit code, and the verbage here should be "issue a warning" not an error.

[comment]: # Outline the design of the solution. Feel free to include ASCII-art diagrams, etc.
## UI/UX Design

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

A suggestion for what could be discussed here: The impact on users that have not used a package manager, will they naturally use update instead of upgrade (or sometimes have issues with distinguishing these two)? And on the other hand, if update was an alias for upgrade, how can similar functionality of upgrade be used with the alias and vice versa? (For example: #120 (comment)) It would be really nice to see what is the plan for dealing with this kind of complexity (update vs. upgrade) from a user standpoint.

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

I prefer "update'. Upgrade has other semantics (at least to me), and update fits more cleanly into what is happening.

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

@JohnMcPMS If you decide to go with update, then I would suggest to make upgrade alias of update to make the life easier for users familiar with other package managers that use upgrade for updating packages.

This comment has been minimized.

@RobGThai

RobGThai Jul 12, 2020

Brew user here, every time I want to update an app, I have to look up help to see if I need to update or upgrade. One is making sure Brew is up to date, the other is for updating the app. If you trigger update without supplying app identifier then Brew will attempt to update every installed packages. Which I'm not a fan of.

Personally, I'm against making alias between update and upgrade. Having just one version of them at the start should reduce confusion on which one to use for newcomers. Instead of doing this, I'd suggest WinGet to only offer just one.

  • winget update - displaying help for update.
  • winget update <package_id> - updating provided package.
  • winget update winget - updating WinGet itself.
  • winget update -a|--all - updating every installed packages.

This comment has been minimized.

@quangkieu

quangkieu Sep 24, 2020

I think winget should use keywork like apt of Debian.

  • Update: for refresh the catalog
  • Upgrade: for actual installation operation of new version

This way we could remove the refresh frequency as winget user does not expect winget run in background as a CLI tool usually run and exit

![show command]()
**Upgrade** when executed by itself will upgrade all apps ready for updates by the Windows Package Manager.

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

Would the user be prompted to confirm the package update? If yes, should there be flag to override this when being used in automation tasks, similar to how other package managers does this?

This comment has been minimized.

@megamorf

megamorf Jun 5, 2020

Sounds like a good idea to stick to that already established pattern where by default a confirmation is required to proceed that can be skipped by passing an additional parameter.

The following options are available.
| Option | Description |

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

What about specifying a manifest file that is located on the user's machine (or another repository)? Would this be part of another spec?

For inno, wix, nullsoft, and exe, the SystemAppId should be a string that is located in either of the Uninstall keys above.
### Upgrade

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

It would be nice to hear how syncing with the remote repository will be handled, will the user have to do this manually (i.e. using an update), or will it be done automatically if the repository hasn't been synced after a said time?

This comment has been minimized.

@KevinLaMS

KevinLaMS Jun 4, 2020
Author Contributor

Martin, I am curious. I think that the syncing of the remote repository is handled via the source:
C:\Users\kevinla>winget source update
Updating all sources...
Updating source: winget...
██████████████████████████████ 100%
Done.

Do you think this addresses the Update concern?

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

It is true you can handle this using the winget source command. I just think the spec should outline how the syncing of the remote repo is handled in context of running an update. Is a user required to run an winget source update before running winget upgrade or will this be implicit from running winget upgrade? (i.e. upgrade will invoke source update)

This comment has been minimized.

@megamorf

megamorf Jun 5, 2020

In Linux package managers update the package cache for all enabled repositories. If a package was installed from repository "foo" then the search for available updates for that specific application will be narrowed down to repository "foo". If you'd actually want to install a new package that is available in multiple repositories without explicitly providing the correct repository name, the package will be installed from the repository with the highest priority. This priority system is also how the upcoming version 3 of PowerShellGet has been designed to define repository ordering.

This comment has been minimized.

@lordcheeto

lordcheeto Jun 11, 2020

As of now, whenever a source (SQLite DB) is opened, it updates the source if it's been more than 5 minutes. winget source update is just the explicit command to update.

// TODO: Enable some amount of user control over this.
constexpr static auto s_DefaultAutoUpdateTime = 5min;
**Update \ Upgrade**
**Uninstall**

### List

This comment has been minimized.

@martinmine

martinmine Jun 4, 2020

Handling available updates with this would be similar to how dotnet handles packages updates, nice 👍 https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-list-package

Co-authored-by: Fabian Meyertöns <[email protected]>
@msftbot msftbot bot removed the Needs-Author-Feedback label Jun 4, 2020
KevinLaMS and others added 6 commits Jun 4, 2020
Co-authored-by: Fabian Meyertöns <[email protected]>
Co-authored-by: Fabian Meyertöns <[email protected]>
Co-authored-by: Fabian Meyertöns <[email protected]>
Co-authored-by: Fabian Meyertöns <[email protected]>
Co-authored-by: Fabian Meyertöns <[email protected]>
@@ -0,0 +1,230 @@
---
author: <first-name> <last-name> <github-id>/<email>

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

Update metadata.

---

# Spec Title

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

Add links to Issues per latest spec template.


![list command](Images\120-List.png)

**List** when executed by itself will show all apps installed by the Windows Package Manager.

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

Do we actually care about differentiating things installed by winget vs things that weren't? Doesn't seem very useful to me.

| **--moniker** | Filter results by application moniker. |
| **--tag** | Filter results by tag
| **--command** | Filter results by command
| **-s,--source** | Find the application using the specified [source](source.md). |

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

Not relevant.



## Mapping between Windows Package Manager repo and Apps and Features
*StoreApps*

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

MSIX. Store is potentially a whole other can of worms. And we might have things available via both (Terminal for instance).

[comment]: # Outline the design of the solution. Feel free to include ASCII-art diagrams, etc.
## UI/UX Design

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

I prefer "update'. Upgrade has other semantics (at least to me), and update fits more cleanly into what is happening.

*Upgrade app when no update is available*
If the user attempts to update an app that there is no known update for, the package manager will issue an error.

This comment has been minimized.

@JohnMcPMS

JohnMcPMS Jun 4, 2020
Member

Yes, I believe so. If the command requested cannot be executed, the process should return a failure exit code.

## Usage
```winget uninstall [[-q] <query>] [<options>]```

This comment has been minimized.

@yao-msft

yao-msft Jun 5, 2020
Contributor

Does uninstall handle apps installed outside of WinGet?
If there're multiple matches with the query. Should we return a warning message saying "please refine the input ..." like what install command does today?

| Argument | Description |
|--------------|-------------|
| **-q,--query** | The query used to search for an application. |
| **-?, --help** | Gets additional help on this command. |

This comment has been minimized.

@dev-nicolaos

dev-nicolaos Jun 16, 2020

Not a huge deal, but I'd personally prefer the -h alias as it is far more common and more clearly maps to --help than -?

This comment has been minimized.

@GauthierPLM

GauthierPLM Jul 8, 2020

Isn't -? the standard on Windows while -h is more a Unix thing?

KevinLaMS added 3 commits Jun 22, 2020
…ade chnages based on internal discussions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.