Skip to content

Disable PublishReadyToRun for framework dependent packages #12450

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.

Sign up for GitHub

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

Jump to bottom
Merged
TravisEz13 merged 2 commits into from
Apr 24, 2020
Merged

Disable PublishReadyToRun for framework dependent packages #12450

TravisEz13 merged 2 commits into from
Apr 24, 2020

Conversation

@adityapatwardhan
Copy link
Member

@adityapatwardhan adityapatwardhan commented Apr 23, 2020 •
edited
Loading

PR Summary

PublishReadyToRun is not supported for a framework dependent package as it needs a runtime to be specified. This PR disables ReadyToRun by overriding the setting Powershell.Common.prop.

PR Context

Fix package build failure introduced by #12361

PR Checklist

@ghost ghost assigned TravisEz13 Apr 23, 2020
PoshChan
PoshChan reviewed Apr 23, 2020
View reviewed changes
iSazonov
iSazonov approved these changes Apr 23, 2020
View reviewed changes
Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

I wonder why packaging test passed previously.

@adityapatwardhan
Copy link
Member Author

adityapatwardhan commented Apr 23, 2020

@iSazonov I believe we do not build fxdependent packages in CI.

PoshChan
PoshChan reviewed Apr 23, 2020
View reviewed changes
PoshChan
PoshChan reviewed Apr 23, 2020
View reviewed changes
@adityapatwardhan
Copy link
Member Author

adityapatwardhan commented Apr 23, 2020

@PoshChan please retry static, windows

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 23, 2020

@adityapatwardhan, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows

PoshChan
PoshChan reviewed Apr 23, 2020
View reviewed changes
build.psm1
$Arguments += "/property:IsWindows=true"
}
else {
$Arguments += "/property:IsWindows=false"
Copy link
Collaborator

@PoshChan PoshChan Apr 23, 2020

Choose a reason for hiding this comment

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

@adityapatwardhan, your last commit had 10 failures in PowerShell-CI-static-analysis
(These are 5 of the failures)

Verify Markdown Links.Verify links in /home/vsts/work/1/s/README.md.https://blogs.msdn.microsoft.com/powershell/2017/01/31/powershell-open-source-community-dashboard/ should work

Connection timed out
at <ScriptBlock>, /home/vsts/work/1/s/test/common/markdown/markdown-link.tests.ps1: line 114
114:                                 $null = Invoke-WebRequest -uri $url -RetryIntervalSec 10 -MaximumRetryCount 6

Verify Markdown Links.Verify links in /home/vsts/work/1/s/README.md.https://blogs.msdn.microsoft.com/powershell/2017/01/31/powershell-open-source-community-dashboard/ should work

Failed to complete request to "https://blogs.msdn.microsoft.com/powershell/2017/01/31/powershell-open-source-community-dashboard/". Response status code does not indicate success: 404 (Not Found).
at <ScriptBlock>, /home/vsts/work/1/s/test/common/markdown/markdown-link.tests.ps1: line 119
119:                                     throw "Failed to complete request to `"$url`". $($_.Exception.Message)"

Verify Markdown Links.Verify links in /home/vsts/work/1/s/demos/WindowsPowerShellModules/README.md.https://blogs.msdn.microsoft.com/powershell/2017/07/14/powershell-6-0-roadmap-coreclr-backwards-compatibility-and-more/ should work

Failed to complete request to "https://blogs.msdn.microsoft.com/powershell/2017/07/14/powershell-6-0-roadmap-coreclr-backwards-compatibility-and-more/". Response status code does not indicate success: 404 (Not Found).
at <ScriptBlock>, /home/vsts/work/1/s/test/common/markdown/markdown-link.tests.ps1: line 119
119:                                     throw "Failed to complete request to `"$url`". $($_.Exception.Message)"

Verify Markdown Links.Verify links in /home/vsts/work/1/s/demos/WindowsPowerShellModules/README.md.https://blogs.msdn.microsoft.com/powershell/2017/07/14/powershell-6-0-roadmap-coreclr-backwards-compatibility-and-more/ should work

Failed to complete request to "https://blogs.msdn.microsoft.com/powershell/2017/07/14/powershell-6-0-roadmap-coreclr-backwards-compatibility-and-more/". Response status code does not indicate success: 404 (Not Found).
at <ScriptBlock>, /home/vsts/work/1/s/test/common/markdown/markdown-link.tests.ps1: line 119
119:                                     throw "Failed to complete request to `"$url`". $($_.Exception.Message)"

Verify Markdown Links.Verify links in /home/vsts/work/1/s/CHANGELOG/6.0.md.https://blogs.msdn.microsoft.com/powershell/2017/07/14/powershell-6-0-roadmap-coreclr-backwards-compatibility-and-more/ should work

Failed to complete request to "https://blogs.msdn.microsoft.com/powershell/2017/07/14/powershell-6-0-roadmap-coreclr-backwards-compatibility-and-more/". Response status code does not indicate success: 404 (Not Found).
at <ScriptBlock>, /home/vsts/work/1/s/test/common/markdown/markdown-link.tests.ps1: line 119
119:                                     throw "Failed to complete request to `"$url`". $($_.Exception.Message)"

TravisEz13
TravisEz13 reviewed Apr 23, 2020
View reviewed changes
@adityapatwardhan
Copy link
Member Author

adityapatwardhan commented Apr 24, 2020

@PoshChan please retry static

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 24, 2020

@adityapatwardhan, successfully started retry of PowerShell-CI-static-analysis

@adityapatwardhan
Copy link
Member Author

adityapatwardhan commented Apr 24, 2020

@TravisEz13 Ready to merge?

@TravisEz13 TravisEz13 merged commit 16ec1cc into PowerShell:master Apr 24, 2020
@TravisEz13 TravisEz13 added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Apr 24, 2020
@TravisEz13 TravisEz13 added this to the 7.1.0-preview.3 milestone Apr 24, 2020
@ghost
Copy link

ghost commented May 19, 2020

🎉v7.1.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

@imba-tjd
Copy link
Contributor

imba-tjd commented Nov 18, 2020 •
edited
Loading

Actually Framework Dependent builds DO support ReadyToRun. Just specify both rid and --self-contained=false.

Personally I always have the latest .net SDK installed, so I don't want to use the self contained release. It would be better in my case to use a win-x64-fxdependent with ReadyToRun enabled comparing to win-fxdependent.

@iSazonov
Copy link
Collaborator

iSazonov commented Nov 18, 2020

@imba-tjd Please open new issue with you feedback. Thanks!

@imba-tjd imba-tjd mentioned this pull request Nov 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 TravisEz13 TravisEz13 left review comments

@iSazonov iSazonov iSazonov approved these changes

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw

+2 more reviewers

@PoshChan PoshChan PoshChan left review comments

@anmenaga anmenaga anmenaga approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@TravisEz13 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

7.1.0-preview.3

Development

Successfully merging this pull request may close these issues.

6 participants

@adityapatwardhan @PoshChan @imba-tjd @iSazonov @TravisEz13 @anmenaga