Skip to content

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

Merged
chamons merged 3 commits into
masterfrom
xm_tfi_mutate
Feb 28, 2019
Merged

[mac][msbuild] Add MigrateToNewXMTFI to convert TFI to new Xamarin.Mac.NET (opt-in)#5654
chamons merged 3 commits into
masterfrom
xm_tfi_mutate

Conversation

@chamons
Copy link
Copy Markdown
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 requested a review from jstedfast February 21, 2019 20:51
@chamons chamons mentioned this pull request Feb 21, 2019
Closed
@monojenkins
Copy link
Copy Markdown
Contributor

monojenkins commented Feb 21, 2019

Build failure
❌ Build was aborted

@monojenkins
Copy link
Copy Markdown
Contributor

monojenkins commented Feb 22, 2019

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
Comment thread 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
Copy Markdown
Member

@rolfbjarne rolfbjarne Feb 22, 2019

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
Copy Markdown
Contributor Author

@chamons chamons Feb 22, 2019

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
Copy Markdown
Contributor Author

@chamons chamons Feb 22, 2019

Choose a reason for hiding this comment

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

MigrateToNewXMIdentifier ?

Copy link
Copy Markdown
Contributor

@kzu kzu Feb 27, 2019

Choose a reason for hiding this comment

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

UseDesktopFrameworkIdentifier?

@monojenkins
Copy link
Copy Markdown
Contributor

monojenkins commented Feb 22, 2019

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
Copy Markdown
Contributor Author

chamons commented Feb 22, 2019

build

@monojenkins
Copy link
Copy Markdown
Contributor

monojenkins commented Feb 22, 2019

Build failure
❌ Build was aborted

🔥 Build failed 🔥

@chamons
Copy link
Copy Markdown
Contributor Author

chamons commented Feb 22, 2019

build

@monojenkins
Copy link
Copy Markdown
Contributor

monojenkins commented Feb 22, 2019

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
Copy Markdown
Contributor Author

chamons commented Feb 25, 2019

@kzu \ @emaf 👍 ?

@chamons
Copy link
Copy Markdown
Contributor Author

chamons commented Feb 25, 2019

Also @jstedfast ?

@kzu
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

@dalexsoto dalexsoto dalexsoto approved these changes

@rolfbjarne rolfbjarne rolfbjarne approved these changes

@jstedfast jstedfast Awaiting requested review from jstedfast

@emaf emaf Awaiting requested review from emaf

@VincentDondain VincentDondain Awaiting requested review from VincentDondain

+3 more reviewers

@kzu kzu kzu left review comments

@spouliot spouliot spouliot approved these changes

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

Reviewers whose approvals may not affect merge requirements

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