Skip to content
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

[mac][msbuild] Add MigrateToNewXMTFI to convert TFI to new Xamarin.Mac.NET (opt-in) #5654

Merged
merged 3 commits into from
Feb 28, 2019
Merged

[mac][msbuild] Add MigrateToNewXMTFI to convert TFI to new Xamarin.Mac.NET (opt-in) #5654

merged 3 commits into from
Feb 28, 2019

Conversation

chamons
Copy link
Contributor

@chamons chamons commented Feb 21, 2019 •
edited by rolfbjarne
Loading

To allow nuget to target XM Full we need to have a unique TFI (TargetFrameworkIdentifier).

However, that's a really scary change to force, so let's opt-in for now. You can set

<MigrateToNewXMTFI>true</MigrateToNewXMTFI>

in your project or

MigrateToNewXMTFI=true msbuild project.csproj

to try it out.

We can convert the opt-in to an opt-out with sufficient validation \ releases.

@chamons
[mac][msbuild] Add MigrateToNewXMTFI to convert TFI to new Xamarin.Ma…
6e7266b 
…c.NET (opt-in)

- #5480
- Related: NuGet/NuGet.Client#2572

To allow nuget to target XM Full we need to have a unique TFI (TargetFrameworkIdentifier).

However, that's a really scary change to force, so let's opt-in for now. You can set

<MigrateToNewXMTFI>true</MigrateToNewXMTFI> in your project or

MigrateToNewXMTFI=true msbuild project.csproj

to try it out. We can convert the opt-in to an opt-out with sufficient validation \ releases.
@chamons chamons mentioned this pull request Feb 21, 2019
Closed
@monojenkins
Copy link
Collaborator

Build failure
❌ Build was aborted

@monojenkins
Copy link
Collaborator

Build failure
❌ Build was aborted

@rolfbjarne rolfbjarne changed the title [mac][msbuild] Add MigrateToNewXMTFI to convert TFI to new Xamarin.Ma… [mac][msbuild] Add MigrateToNewXMTFI to convert TFI to new Xamarin.Mac.NET (opt-in) Feb 22, 2019
rolfbjarne
rolfbjarne reviewed Feb 22, 2019
View reviewed changes
msbuild/Xamarin.Mac.Tasks/Xamarin.Mac.Common.props Outdated
Now that we're getting "real" nuget support, we need to stop lying without breaking the world. We're starting by
allowing "opt-in" MigrateToNewXMTFI which will swap your declared TFI to the new hotness.
-->
<PropertyGroup Condition=" '$(MigrateToNewXMTFI)' == 'true' And '$(TargetFrameworkIdentifier)' != 'Xamarin.Mac' And '$(UseXamMacFullFramework)' == 'true' ">
Copy link
Member

Choose a reason for hiding this comment

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

I don't really like abbreviations, so maybe name the variable something like: UseXamarinMacTargetFrameworkIdentifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TFI (TargetFrameworkIdentifier) and TFV (TargetFrameworkVersion) are pretty standard msbuild jargon from what i can tell, but I concede the point. Let me think of a better name...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MigrateToNewXMIdentifier ?

Copy link
Contributor

Choose a reason for hiding this comment

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

UseDesktopFrameworkIdentifier?

@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
✅ Generator Diff (no change)
🔥 Test run failed 🔥

Test results

14 tests failed, 0 tests skipped, 69 tests passed.

Failed tests

  • xammac tests/Mac/Debug: Failed (Test run failed.)
  • xammac tests/Mac/Release: Failed (Test run failed.)
  • monotouch-test/iOS Unified 32-bits - simulator/Debug: Failed
  • link sdk/iOS Unified 32-bits - simulator/Debug: Failed
  • link sdk/iOS Unified 32-bits - simulator/Release: Failed
  • monotouch-test/iOS Unified 32-bits - simulator/Debug (static registrar): Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug: Failed
  • link sdk/iOS Unified 64-bits - simulator/Debug: Failed
  • link sdk/iOS Unified 64-bits - simulator/Release: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (static registrar): Failed
  • monotouch-test/tvOS - simulator/Debug: Failed
  • link sdk/tvOS - simulator/Debug: Failed
  • link sdk/tvOS - simulator/Release: Failed
  • monotouch-test/tvOS - simulator/Debug (static registrar): Failed

@chamons
Copy link
Contributor Author

chamons commented Feb 22, 2019

build

@monojenkins
Copy link
Collaborator

Build failure
❌ Build was aborted

🔥 Build failed 🔥

@chamons
Copy link
Contributor Author

chamons commented Feb 22, 2019

build

@monojenkins
Copy link
Collaborator

Build success
✅ Build succeeded
✅ API Diff (from stable)
ℹ️ API Diff (from PR only) (please review changes)
✅ Generator Diff (no change)
✅ Test run succeeded

@chamons
Copy link
Contributor Author

chamons commented Feb 25, 2019

@kzu \ @emaf 👍 ?

@chamons
Copy link
Contributor Author

chamons commented Feb 25, 2019

Also @jstedfast ?

@kzu
Copy link
Contributor

kzu commented Feb 27, 2019 •
edited
Loading

Just saw the related link to the nuget issue that implements the mapping, should be 👍 once that ships.

kzu
kzu reviewed Feb 27, 2019
View reviewed changes
Copy link
Contributor

@kzu kzu left a comment

Choose a reason for hiding this comment

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

Should be fine, but would require testing on VS to see how the project system works with an updated XM and NuGet.

@chamons
Copy link
Contributor Author

chamons commented Feb 27, 2019

Right now VS Mac has zero support for building XM Full so a XM Full project w\ this tag would really be unsupported.

Not a bad idea to see how it freaks out though.

@chamons chamons merged commit 2df9e4c into master Feb 28, 2019
@chamons chamons deleted the xm_tfi_mutate branch February 28, 2019 22:33
@chamons chamons mentioned this pull request Feb 28, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kzu kzu kzu left review comments

@mandel-macaque mandel-macaque mandel-macaque approved these changes

@spouliot spouliot spouliot approved these changes

@rolfbjarne rolfbjarne rolfbjarne approved these changes

@dalexsoto dalexsoto dalexsoto approved these changes

@jstedfast jstedfast Awaiting requested review from jstedfast

@emaf emaf Awaiting requested review from emaf

@VincentDondain VincentDondain Awaiting requested review from VincentDondain

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@chamons @monojenkins @kzu @dalexsoto @rolfbjarne @spouliot @mandel-macaque