Skip to content

Do not ship fullclr binaries of PackageManagement - #8700

Merged
Travis Plunk (TravisEz13) merged 4 commits into
PowerShell:masterfrom
bergmeister:RemovePSGetFullClr
Jan 22, 2019
Merged

Do not ship fullclr binaries of PackageManagement#8700
Travis Plunk (TravisEz13) merged 4 commits into
PowerShell:masterfrom
bergmeister:RemovePSGetFullClr

Conversation

@bergmeister

@bergmeister Christoph Bergmeister (bergmeister) commented Jan 19, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

Related: #8699

PR Context

Because PowerShellGet does not support publishing/saving module on a per platform basis (see this issue), PowerShell currently also ships the fullclr binaries of PackageManagement, which it does not need. Therefore removing it to minimise the package size, this saves 1.19 MB.

PR Checklist

@bergmeister Christoph Bergmeister (bergmeister) changed the title Remove fullclr binaries of PowerShellGet from WiX installer WIP Remove fullclr binaries of PowerShellGet from WiX installer Jan 19, 2019
@bergmeister Christoph Bergmeister (bergmeister) changed the title WIP Remove fullclr binaries of PowerShellGet from WiX installer WIP Remove fullclr binaries of PackageManagement from WiX installer Jan 19, 2019
@bergmeister Christoph Bergmeister (bergmeister) changed the title WIP Remove fullclr binaries of PackageManagement from WiX installer WIP Do not ship fullclr binaries of PackageManagement Jan 19, 2019
@bergmeister Christoph Bergmeister (bergmeister) changed the title WIP Do not ship fullclr binaries of PackageManagement Do not ship fullclr binaries of PackageManagement Jan 19, 2019
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Jan 21, 2019
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@iSazonov

Ilya (iSazonov) commented Jan 21, 2019

Copy link
Copy Markdown
Collaborator

I guess that we break in-place upgrade (we will not remove the trash on file system) if removing the components.

@daxian-dbw

Dongbo Wang (daxian-dbw) commented Jan 21, 2019

Copy link
Copy Markdown
Member

Hmm, definitely need Travis Plunk (@TravisEz13)'s approval on this.

@bergmeister

Christoph Bergmeister (bergmeister) commented Jan 21, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Ilya (@iSazonov) Why would the MSI upgrade not remove the old component? And even if it did not remove it, then there would still not be a problem since this folder is not needed/useful for PowerShell Core at all. In the related issue #8699, I also describe that the next version of Packagemanagement will ship with a netstandard2.0 folder that replaces the current 2 folders netcoreapp2.0 and netstandard1.6, so we will need to tackle this problem anyway.

@iSazonov

Ilya (iSazonov) commented Jan 22, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator

We already had a problem with the shortcut key in the registry so we should be careful here.
Also we should consider a patch update scenario.

Steve Lee (@SteveL-MSFT) Travis Plunk (@TravisEz13) what do you think?

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Jan 22, 2019

Copy link
Copy Markdown
Member

Seems fine to me. Even if the existing assemblies don't get removed, it didn't make it worse.

@TravisEz13

Travis Plunk (TravisEz13) commented Jan 22, 2019

Copy link
Copy Markdown
Member

We should not consider a patch upgrade scenario during a preview. This change should not be back ported to 6.1, but it is fine to introduce during the preview process.

@TravisEz13
Travis Plunk (TravisEz13) merged commit beae569 into PowerShell:master Jan 22, 2019
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Jan 23, 2019
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
@bergmeister @TravisEz13
Do not ship fullclr binaries of PackageManagement (PowerShell#8700)
1ffc29a 
## PR Summary

Related: PowerShell#8699

## PR Context  

Because `PowerShellGet` does not support publishing/saving module on a per platform basis (see [this](https://github.com/PowerShell/PowerShellGet/issues/273) issue), PowerShell currently also ships the fullclr binaries of `PackageManagement`, which it does not need. Therefore removing it to minimise the package size, this saves 1.19 MB.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@TravisEz13 Travis Plunk (TravisEz13) TravisEz13 approved these changes
@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@anmenaga Andrew (anmenaga) Awaiting requested review from anmenaga
@PaulHigin Paul Higinbotham (PaulHigin) Awaiting requested review from PaulHigin

Assignees

@TravisEz13 Travis Plunk (TravisEz13)

Labels

CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@bergmeister @iSazonov @daxian-dbw @SteveL-MSFT @TravisEz13 @adityapatwardhan