-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Download command #3376
Download command #3376
Conversation
… downloadCommand
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Workflow::ReportDependencies(Resource::String::InstallAndUpgradeCommandsReportDependencies) << | ||
Workflow::EnableWindowsFeaturesDependencies << | ||
Workflow::ManagePackageDependencies(Resource::String::InstallAndUpgradeCommandsReportDependencies); | ||
Workflow::ReportDependencies(Resource::String::PackageRequiresDependencies) << |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are probably reporting dependencies twice, try install a package with dependencies to confirm
Update ProcessMultiplePakckages to not report dependencies if m_ignoreDependencies is true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Report dependencies needs to be called here and inside 'ProcessMultiplePackages'. I verified the behavior and it doesn't report dependencies of the same package twice.
The first time ReportDependencies is called is to report the dependencies of the main package. The second time ReportDependencies is called is to the report the dependencies of a dependency package.
This scenario is also utilized by the import command to report dependencies for each package in the subcontexts. If we update ProcessMultiplePackages to not report dependencies if m_ignoreDependencies is true, the import command would fail to display the report dependencies message to the user for each imported package.
installContext << Workflow::ManagePackageDependencies(m_dependenciesReportMessage); | ||
currentContext << | ||
Workflow::CreateDependencySubContexts(m_dependenciesReportMessage, m_includeInstalledPackages) << | ||
Workflow::ProcessMultiplePackages(m_dependenciesReportMessage, APPINSTALLER_CLI_ERROR_INSTALL_DEPENDENCIES, {}, false, true, true, true, m_includeInstalledPackages, m_downloadInstallerOnly); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the case for winget import and winget upgrade --all
Here, only package dependencies are considered, we should update the logic to include windows features as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added logic to include support for windows features.
m_refreshPathVariable(refreshPathVariable){} | ||
m_refreshPathVariable(refreshPathVariable), | ||
m_includeInstalledPackages(includeInstalledPackages), | ||
m_downloadInstallerOnly(downloadInstallerOnly){} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For m_includeInstalledPackages and m_downloadInstallerOnly. As ProcessMultiplePackages is a "one for all" type of workflow, we could just read the context flag and determine the behavior accordingly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed these members as they are not needed. Changed to reading the context flag and determining the bheavior accordingly.
I haven't looked at the code yet, but does this work for the msstore source? If so, can we do something different from than |
This code works for win32 apps from msstore. I'll be extending it to work with msstore types later. Maybe to improve it a bit, if version is unknown, we do not concatenate it. Id is the only unique identifier per source. Maybe we can use the id for the directory and PackageName for the installer name? |
// This must be done before any co_awaits since it requires info from the rpc caller thread. | ||
auto [hrGetCallerId, callerProcessId] = GetCallerProcessId(); | ||
WINGET_RETURN_DOWNLOAD_RESULT_HR_IF_FAILED(hrGetCallerId); | ||
WINGET_RETURN_DOWNLOAD_RESULT_HR_IF_FAILED(EnsureProcessHasCapability(Capability::PackageManagement, callerProcessId)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, check any one of PackageManagement and PackageQuery exists
If this isn't the right place to ask, feel free to delete it.
The spec also mentioned a parameter "--fileName". Has this been implemented as well? |
There may be more than one packages downloaded if dependencies are included, and we think current naming strategy is unique enough to prevent confliction, so we removed the --file-name arg. Spec will be updated once the final implementation is done. |
Good point, that could be a conflict. I guess I'll try to rename the files in my workflow with another part of my script then. |
Related to: #2953
Implements the new
winget download
command.Changes:
download
command with arguments to allow users to download any installer from a package manifest.installer-type
argument to allow users to select which installertype they would like to download.%USERPROFILE%/Downloads/{packageId}.{Version}
folder. Users can specify the exact download directory by using the--download-directory
argument.Dependencies
so that users can differentiate between the main installer and package dependency installersManagePackageDependencies
has been refactored intoCreateDependencySubContexts
which is now only responsible for building the dependency graph and populating the context with the package sub contexts execution data.InstallDependencies
workflow generates the package dependency sub contexts and installs the package dependenciesDownloadDependencies
workflow generates the package dependency sub contexts and downloads the package dependenciesInstallDependenciesFromCOM
only installs the package dependencies (since package dependency sub contexts should have already been created by calling theCOMDownloadCommand
).Tests:
Microsoft Reviewers: codeflow:open?pullrequest=#3376