microsoft
Add Scope Switches to Manifest
- [x] I have signed the Contributor License Agreement.
- [ ] This pull request is related to an issue.
Some installer types, like Inno, have default switches that control whether they install in User scope or Machine scope. Other times, ISV's build custom switches into their installer to allow for this.
Currently, in order to have separate switches for each scope, there must be two installer nodes. One for the user scope with the Custom switch set to the user scope switch, and one for the machine scope with the Custom switch similarly set. This makes it difficult to suggest changes when a user only has one installer in the manifest. Additionally, it makes it difficult to identify the differences between installers when they are identical except for the Scope and the Custom switch.
This PR adds two new installer switches - ScopeUser which is added when the installation is happening on a user scoped install, and ScopeMachine which is added when the installation is happening on a machine scoped install. It also sets the defaults for Inno installers.
This means that two scoped installers can now be combined into a single un-scoped installer in the manifest. As this will appear as ScopeEnum::Unknown for manifest selection, the logic when loading a manifest was updated such that if ScopeUser or ScopeMachine are present for an installer, then a copy of that installer for each scope is loaded. This allows for certainty that the correct scope will be used in an upgrade scenario, as the ManifestComparator will choose the scope which is currently installed.
Microsoft.Azure.StorageExplorer manifest
| Old | New |
|
|
MyCloudGame.AirGamePlay manifest
| Old | New |
|
|
@check-spelling-bot Report
:red_circle: Please review
See the :open_file_folder: files view or the :scroll:action log for details.
Unrecognized words (1)
mch
Previously acknowledged words that are now absent
bitspace Mta PFM testdata :arrow_right:To accept :heavy_check_mark: these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands
... in a clone of the [email protected]:Trenly/winget-cli.git repository
on the ScopeSwitches branch (:information_source: how do I use this?):
curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/winget-cli/actions/runs/7893560406/attempts/1'
Available :books: dictionaries could cover words not in the :blue_book: dictionary
This includes both expected items (527) from .github/actions/spelling/expect.txt and unrecognized words (1)
| Dictionary | Entries | Covers |
|---|---|---|
| cspell:win32/src/win32.txt | 53509 | 20 |
| cspell:python/src/python/python-lib.txt | 3873 | 3 |
| cspell:python/src/python/python.txt | 453 | 2 |
| cspell:python/src/common/extra.txt | 741 | 2 |
| cspell:php/php.txt | 2597 | 2 |
| cspell:npm/npm.txt | 288 | 2 |
| cspell:java/java.txt | 7642 | 2 |
| cspell:django/django.txt | 859 | 2 |
| cspell:csharp/csharp.txt | 19 | 2 |
| cspell:sql/src/tsql.txt | 455 | 1 |
Consider adding them using (in .github/workflows/spelling3.yml):
with:
extra_dictionaries:
cspell:win32/src/win32.txt
cspell:python/src/python/python-lib.txt
cspell:python/src/python/python.txt
cspell:python/src/common/extra.txt
cspell:php/php.txt
cspell:npm/npm.txt
cspell:java/java.txt
cspell:django/django.txt
cspell:csharp/csharp.txt
cspell:sql/src/tsql.txt
To stop checking additional dictionaries, add:
with:
check_extra_dictionaries: ''
If the flagged items are :exploding_head: false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it, try adding it to the
patterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Winget 1.7 manifest is closed for changes as we are doing a winget 1.7 release candidate by end of this week. We can consider this change in winget 1.8 if needed.
P.S. Personally I feel these switches are a bit overlapping with the InstallerScope installer property. In the whole workflow, InstallerScope is used for installer selection, like architecture. And specific switch values for the InstallerScope, architecture would go to Custom (existing winget-pkgs usage). ScopeUser and ScopeMachine seem a bit inconsistent with current switches pattern. In this change, we are listing switches for a specific installer property value. It feels like as if an installer supports both x64/x86 installation, then we are adding x64 switches and x86 switches. I think one advantage with this change is that we can prepopulate these switches with known installer types more easily. We can evaluate the usage of these switches and then make a decision.
Personally I feel these switches are a bit overlapping with the InstallerScope installer property.
They are, but the advantage of having these be switches can make it easier to understand how an installer works, and that the switches are the only difference between the scopes.
In the whole workflow, InstallerScope is used for installer selection, like architecture.
This change may partially address one key issue though, in that after an installer is selected, if the scope is not specified in the manifest or through an installer switch, there is not a way to know which scope has been selected for install. Consider the upgrade case where an installer is of unknown scope, and the current install is machine scoped. Once the installer is selected, there is no easy way to determine the scope that should be used. By allowing there to be a concept of switches for scope, the loading of the single installer into two with defined scopes solves that issue.
ScopeUser and ScopeMachine seem a bit inconsistent with current switches pattern. In this change, we are listing switches for a specific installer property value. It feels like as if an installer supports both x64/x86 installation, then we are adding x64 switches and x86 switches.
I can understand that; These could be made internal-only, but then they couldn’t be overridden. They could also be renamed, which may help.
Regarding the switches for architecture, I would argue that if they were anywhere near as common as switches for scope, then why not have them switch-selectable with defaults based on installer type? More than 10% of the manifests currently in winget-pkgs currently have /CURRENTUSER specified, and I'm sure there are other switches that are used for specifying scope. Architecture, however, is usually based on the current system architecture and not a switch. Not that it couldn’t be a switch in some cases, but it is significantly less common
By the way, we need to also consider "somewhat breaking" behavior change for old clients. For the example with the Microsoft.Azure.StorageExplorer, if a winget 1.6 client is working on the new manifest, it does not know to look at the new switches, it relies on the installer selection logic to apply correct switches. This change will make old clients see only 1 installer with Custom: /NoRestart.
By the way, we need to also consider "somewhat breaking" behavior change for old clients. For the example with the Microsoft.Azure.StorageExplorer, if a winget 1.6 client is working on the new manifest, it does not know to look at the new switches, it relies on the installer selection logic to apply correct switches. This change will make old clients see only 1 installer with Custom: /NoRestart.
That is true, but to me it is no different than updating the default switches for an installer type. In an older version of the client, the updated default switches wouldn’t be applied anyways which would result in the same behavior if the manifest didn’t include them. It's just that this happens to be the scope switches specifically.
If this is something that can be moved into 1.8, I would imagine that the same process would be used where the new manifest version wouldn’t be accepted in the winget-pkgs repo until a substantial portion of users had upgraded to the 1.8 client
Also, if this is something that the team decides to simply not move forward on, its no big deal to me. I didn't put too much effort into making these changes and only raised the PR as I thought it could be helpful to manifest authors
Also, if this is something that the team decides to simply not move forward on, its no big deal to me. I didn't put too much effort into making these changes and only raised the PR as I thought it could be helpful to manifest authors
Yep, thanks for raising this idea. I have mixed feeling about having specific "installer property value" based switches. I think we'll revisit in winget 1.8.
By the way, we need to also consider "somewhat breaking" behavior change for old clients. For the example with the Microsoft.Azure.StorageExplorer, if a winget 1.6 client is working on the new manifest, it does not know to look at the new switches, it relies on the installer selection logic to apply correct switches. This change will make old clients see only 1 installer with Custom: /NoRestart.
That is true, but to me it is no different than updating the default switches for an installer type. In an older version of the client, the updated default switches wouldn’t be applied anyways which would result in the same behavior if the manifest didn’t include them. It's just that this happens to be the scope switches specifically.
If this is something that can be moved into 1.8, I would imagine that the same process would be used where the new manifest version wouldn’t be accepted in the winget-pkgs repo until a substantial portion of users had upgraded to the 1.8 client
I feel it's a bit different. "Updating default switches" does not break existing behavior of old clients. Old clients just do not get new feature. While "manifest change in winget-pkgs" changes existing behavior of old clients. But if we decide to go with this change, we could certainly wait for some time and update manifests in winget-pkgs after large portion of users have moved to new clients.
I feel it's a bit different. "Updating default switches" does not break existing behavior of old clients. Old clients just do not get new feature. While "manifest change in winget-pkgs" changes existing behavior of old clients. But if we decide to go with this change, we could certainly wait for some time and update manifests in winget-pkgs after large portion of users have moved to new clients.
Take for example the new default switches that were just added for Inno - specifically /SUPPRESSMSGBOXES. There are several manifests that specify that as a custom switch. If a contributor creates a new manifest, tests with the new client, and leaves out that custom switch, it has effectively "broken" the old client as it won't be able to install silently. This would be the case as soon as 1.7 becomes the version used in the validation pipelines.
Sure, adding the /SUPPRESSMSGBOXES is much less of a change, but it still has equal potential for older clients to not properly handle new manifests, depending on how they were created?
Take for example the new default switches that were just added for Inno - specifically
/SUPPRESSMSGBOXES. There are several manifests that specify that as a custom switch. If a contributor creates a new manifest, tests with the new client, and leaves out that custom switch, it has effectively "broken" the old client as it won't be able to install silently. This would be the case as soon as 1.7 becomes the version used in the validation pipelines.Sure, adding the
/SUPPRESSMSGBOXESis much less of a change, but it still has equal potential for older clients to not properly handle new manifests, depending on how they were created?
I agree, any installer switches changes to existing manifests may affect existing behavior for old clients. Do you happen to know if specifying "/SUPPRESSMSGBOXES" twice during installer invocation will cause installer failures? If not, maybe we should just not modify existing manifests that have "/SUPPRESSMSGBOXES" as Custom switch. By the way, I thought /SUPPRESSMSGBOXES would better fit in silent | interactive behavior switches, not Custom. So that the /SUPPRESSMSGBOXES change would not affect existing manifests. But I cannot control how every manifest is authored.
Regarding old clients handling new manifests, yes, there are potential that old clients cannot work well with new manifests. Our goal is to keep compatibility as best as we can. Like for this change, if we know moving the scope switches will break old clients, we should probably not do anything to existing manifests. Or only make the manifest change after most people moved to new clients.