Skip to content

Revert "Revert nuget to 2.0.200609.3" #796

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
jlaanstra merged 5 commits into from
Dec 15, 2020
Merged

Revert "Revert nuget to 2.0.200609.3" #796

jlaanstra merged 5 commits into from
Dec 15, 2020

Conversation

@jlaanstra
Copy link
Contributor

@jlaanstra jlaanstra commented Nov 16, 2020 •
edited
Loading

Reverts #793 and adds the change from #794

Validate:

  • OS
  • Your Phone
  • Terminal
  • WinUI

kennykerr
kennykerr requested changes Nov 16, 2020
View reviewed changes
Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

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

Waiting for validation. 😉

ChrisGuzak
ChrisGuzak approved these changes Nov 16, 2020
View reviewed changes
nuget/CppWinrtRules.Project.xml Outdated

<BoolProperty Name="CppWinRTEnableDefaultPrivateFalse"
DisplayName="Enable Copy Local Defaults"
Description="Enables or disables the default for copying binaries to the output folder"
Copy link
Member

@ChrisGuzak ChrisGuzak Nov 16, 2020

Choose a reason for hiding this comment

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

Consider describing an example scenario where this would be useful.

test/nuget/ConsoleApplication1/ConsoleApplication1Class.idl
@@ -0,0 +1,9 @@
namespace ConsoleApplication1
{
[default_interface]
Copy link
Member

@ChrisGuzak ChrisGuzak Nov 16, 2020

Choose a reason for hiding this comment

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

remove, not needed (this class has a method)

test/nuget/ConsoleApplication1/readme.txt
@@ -0,0 +1,30 @@
========================================================================
Copy link
Member

@ChrisGuzak ChrisGuzak Nov 16, 2020

Choose a reason for hiding this comment

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

consider deleting this file. does not help anyone

@kennykerr
Copy link
Collaborator

kennykerr commented Nov 17, 2020

@ChrisGuzak does this PR address #790?

@kennykerr
Copy link
Collaborator

kennykerr commented Nov 17, 2020

@AlexBAV does this PR address #785?

@AlexBAV
Copy link

AlexBAV commented Nov 18, 2020

@AlexBAV does this PR address #785?

It looks like I found the reason for behavior described in #785. I've provided more information in the issue.

@jlaanstra
Copy link
Contributor Author

jlaanstra commented Nov 18, 2020

I'll probably won't be able to finish all the validation this week and I'm OOF next week. I'll finish this after Thanksgiving.

@kennykerr kennykerr mentioned this pull request Dec 1, 2020
Closed
@kennykerr
Copy link
Collaborator

kennykerr commented Dec 3, 2020

@jbrianceau does this PR address #809

@jbrianceau
Copy link
Member

jbrianceau commented Dec 3, 2020

@jbrianceau Julien Brianceau FTE does this PR address #809

No, I tried with the current 2.0.201113.7 nuget and overwrite CppWinrtRules.Project.xml, Microsoft.Windows.CppWinRT.props and Microsoft.Windows.CppWinRT.targets files using the ones from this PR and the problem is still there when trying to create the app package:

CSC : error CS0006: Metadata file 'D:\Work\CppWinRT_Test\App1\bin\x86\Debug\x64\RuntimeComponent1\RuntimeComponent1.winmd' could not be found [D:\Work\CppWinRT_Test\App1\App1.csproj]

@kennykerr kennykerr mentioned this pull request Dec 3, 2020
Closed
@kennykerr
Copy link
Collaborator

kennykerr commented Dec 3, 2020 •
edited
Loading

@FrayxRulez does this PR address #813?

@jlaanstra
Copy link
Contributor Author

jlaanstra commented Dec 3, 2020

OS validation passed pending the fix for OS#30952588

@jlaanstra
Copy link
Contributor Author

jlaanstra commented Dec 3, 2020

WinUI build passed. Looking into a build error in Terminal.

@FrayxRulez
Copy link

FrayxRulez commented Dec 4, 2020

@FrayxRulez does this PR address #813?

Any easy way to try out?

@jlaanstra
Copy link
Contributor Author

jlaanstra commented Dec 4, 2020

@FrayxRulez does this PR address #813?

Any easy way to try out?

Find the package on disk. Backup the props and targets files under the build folder. Replace those files with the ones from this PR.

@jlaanstra jlaanstra requested a review from kennykerr December 11, 2020 01:25
@jlaanstra
Copy link
Contributor Author

jlaanstra commented Dec 11, 2020

@kennykerr All listed projects have passed validation with these changes.

@kennykerr
Copy link
Collaborator

kennykerr commented Dec 11, 2020

All listed projects have passed validation with these changes.

Excellent! Just to clarify, did you test just the msbuild changes or cppwinrt as well?

@ChrisGuzak, @jbrianceau can you confirm this update works for you?

@jlaanstra
Copy link
Contributor Author

jlaanstra commented Dec 11, 2020

All listed projects have passed validation with these changes.

Excellent! Just to clarify, did you test just the msbuild changes or cppwinrt as well?

@ChrisGuzak, @jbrianceau can you confirm this update works for you?

I did just test the MSBuild changes, by manually replacing the props/targets files.

@jlaanstra
Copy link
Contributor Author

jlaanstra commented Dec 11, 2020

Note that this doesn't contain the fix for #809 yet. I'll be looking into that next.

@kennykerr
Copy link
Collaborator

kennykerr commented Dec 11, 2020

I did just test the MSBuild changes, by manually replacing the props/targets files.

That's fine - I just got the latest cppwinrt working in the OS build so we should be good.

@kennykerr
Copy link
Collaborator

kennykerr commented Dec 11, 2020

Note that this doesn't contain the fix for #809 yet. I'll be looking into that next.

Are you planning on doing that in this PR? Note that this will likely require re-validating everything. 😞

@jlaanstra
Copy link
Contributor Author

jlaanstra commented Dec 15, 2020

Note that this doesn't contain the fix for #809 yet. I'll be looking into that next.

Are you planning on doing that in this PR? Note that this will likely require re-validating everything. 😞

Fix is in #824. I'll merge this one and then do another round of validation with both changes.

@jlaanstra jlaanstra merged commit ac252b2 into master Dec 15, 2020
@jlaanstra jlaanstra deleted the revert-793-nuget branch December 15, 2020 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChrisGuzak ChrisGuzak ChrisGuzak approved these changes

@kennykerr kennykerr kennykerr approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jlaanstra @kennykerr @AlexBAV @jbrianceau @FrayxRulez @ChrisGuzak