-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add ability to package non-release packages #4509
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
Conversation
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 believe .NET CLI generates portable pdb by default on Linux/OSX. See https://github.com/OmniSharp/omnisharp-vscode/wiki/Portable-PDBs#how-to-generate-portable-pdbs.
So the LinuxDebug may not be necessary.
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.
Yes, even for Windows the PDBs that are generated are portable. Hence, for code coverage we explicitly make the DebugType as full, which is Windows style full PDBs.
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.
PS > dir ./powershell/src/powershell-unix/bin/Linux/netcoreapp2.0/osx.10.12-x64/publish/*.pdb -Recurse
Directory: /Users/travisplunk/git/powershell/src/powershell-unix/bin/Linux/netcoreapp2.0/osx.10.12-x64/publish
Mode LastWriteTime Length Name
---- ------------- ------ ----
------ 8/7/17 9:57 AM 236 Microsoft.PowerShell.SDK.pdb
------ 8/7/17 9:57 AM 488 powershell.pdb
PS > dir ./src/powershell-unix/bin/LinuxDebug/netcoreapp2.0/osx.10.12-x64/publish/*.pdb -Recurse
Directory: /Users/travisplunk/git/powershell/src/powershell-unix/bin/LinuxDebug/netcoreapp2.0/osx.10.12-x64/publish
Mode LastWriteTime Length Name
---- ------------- ------ ----
------ 8/5/17 9:10 AM 184768 Microsoft.PowerShell.Commands.Management.pdb
------ 8/5/17 9:10 AM 242416 Microsoft.PowerShell.Commands.Utility.pdb
------ 8/5/17 9:10 AM 78996 Microsoft.PowerShell.ConsoleHost.pdb
------ 8/5/17 9:10 AM 29848 Microsoft.PowerShell.CoreCLR.AssemblyLoadContext.pdb
------ 8/5/17 9:10 AM 39696 Microsoft.PowerShell.CoreCLR.Eventing.pdb
------ 8/5/17 9:10 AM 87268 Microsoft.PowerShell.PSReadLine.pdb
------ 8/5/17 9:10 AM 236 Microsoft.PowerShell.SDK.pdb
------ 8/5/17 9:10 AM 42868 Microsoft.PowerShell.Security.pdb
------ 8/5/17 11:42 AM 488 powershell.pdb
------ 8/5/17 9:10 AM 2916020 System.Management.Automation.pdb
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.
If they are generated, they are not included in the publish directory.
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.
They are generated. However, when specifying -Crossgen, those .pdb files will be removed during Start-Crossgen.
For the same reason, you shouldn't specify -Crossgen when packing debug packages.
tools/packaging/packaging.psm1
Outdated
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.
Just want to double check -- we don't change the existing names of the release packages, right?
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.
Yes, but, I'll update this code, as when I wrote it I assumed Release was the only Release configuration
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.
Yes, even for Windows the PDBs that are generated are portable. Hence, for code coverage we explicitly make the DebugType as full, which is Windows style full PDBs.
82b6d30 to
3bfa807
Compare
|
I changed this to only allow packaging with symbols for now by removing crossgen from the build. I've updated the PR Description. |
|
@adityapatwardhan The comments are addressed. Can you please take another look? |
|
@adityapatwardhan any update? |
adityapatwardhan
left a comment
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.
LGTM
For certain testing, we need builds with symbols. Adding the ability to package builds with symbols for this purpose
I enabled for all packaging types except AppImage.
I forced the option to add
symbolsinto the package name, except for Release/Linux (the Linux Release configuration). This will allow this special package type to be easily identified.