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.

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

Added net6.0-windows on WPF and WinForms and workaround MSBuild.Sdk.Extras issue #260

Merged
merged 9 commits into from
Dec 14, 2021

Conversation

jeremyVignelles
Copy link
Collaborator

Description of Change

  • Added net6.0-windows TFM on LibVLCSharp.WinForms
  • Added net6.0-windows TFM on LibVLCSharp.WPF
  • Added DESKTOP constant to replace the (flaky) NETFRAMEWORK || NETSTANDARD || NET6_0

Issues Resolved

Users referencing LVS.WinForms in .net6.0 should have a working project now

API Changes

None (Fixes missing APIs in 3.6.3)

Platforms Affected

  • Core (all platforms)

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

PR Checklist

  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

jeremyVignelles and others added 5 commits December 9, 2021 17:31
multi targeting new and legacy TFMs is not supported on mac,
so we don't build net6 TFMs on mac and with our samples having project references,
this fails the samples build.
@mfkl
Copy link
Member

mfkl commented Dec 10, 2021

In my opinion the netcore sample (which should be renamed to desktop?) is a good test to multi target these 3 TFMs in an app project, so we should keep my last commit (which is failing the build).

As for the build error:

2021-12-10T12:54:22.1370769Z   D:\a\1\s\samples\LibVLCSharp.NetCore.Sample\Program.cs(14,32): error CS0118: 'MediaPlayer' is a namespace but is used like a type [D:\a\1\s\samples\LibVLCSharp.NetCore.Sample\LibVLCSharp.NetCore.Sample.csproj]

I repro that build error locally, it's a weird one I've seen before sometimes happening. Now we repro consistently.

Replacing MediaPlayer by Shared.MediaPlayer fixes it (but that should not be needed), though it leads to new publishing errors, likely caused by our LibVLC.Windows targets conflicting with the new net6.0 targets when multiple are used in a single app project (which should be supported, right?).

Additionally, we should try to replace all the samples csproj still using the old-style SDK and see if we could use the new style, as well as multi target new and legacy TFMs if possible (i.e. the android sample would target both monoandroid81/monoandroid90 and net6.0-android).

This would help prevent what just happened with the 3.6.2/3.6.3 releases (I take full responsibility).

@jeremyVignelles jeremyVignelles mentioned this pull request Dec 12, 2021
2 tasks
@jeremyVignelles
Copy link
Collaborator Author

Replacing MediaPlayer by Shared.MediaPlayer fixes it (but that should not be needed)

Is there any namespace somewhere that's named MediaPlayer and that conflicts with the type ? Do you have more info on this ? why did that not break before ? (different SDK ?)

@mfkl
Copy link
Member

mfkl commented Dec 13, 2021

Replacing MediaPlayer by Shared.MediaPlayer fixes it (but that should not be needed)

Is there any namespace somewhere that's named MediaPlayer and that conflicts with the type ? Do you have more info on this ? why did that not break before ? (different SDK ?)

SDK issue I'd say. Removing the macos target fixes it (though the build error does not prevent the DLL to be built). Specific to mac/ios toolchain/SDK https://code.videolan.org/videolan/LibVLCSharp/-/issues/326

-   <TargetFrameworks Condition="$([MSBuild]::IsOsPlatform('Windows'))">net6.0;net6.0-windows;net6.0-macos</TargetFrameworks>
+   <TargetFrameworks Condition="$([MSBuild]::IsOsPlatform('Windows'))">net6.0;net6.0-windows</TargetFrameworks>

@mfkl
Copy link
Member

mfkl commented Dec 13, 2021

For WinForms and WPF samples, this change is required for net6.0 (breaks legacy TFMs)

- <Project Sdk="Microsoft.NET.Sdk.WindowsDesktop">
+ <Project Sdk="Microsoft.NET.Sdk">

Cross-targetting for our "internal" samples project seems like a lot of overhead now. Maybe we should just keep net6.0 / latest for these test samples?

@vlc-mirrorer vlc-mirrorer merged commit 5459150 into videolan:3.x Dec 14, 2021
@mfkl
Copy link
Member

mfkl commented Dec 14, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants