Remove framework targets `net40`, `netstandard1.3`, `netstandard1.5` #495

stakx · 2020-05-01T18:09:44Z

This is a follow-up to #407 (comment).

~~net35~~ — NOTE: already removed in Fix broken Linux CI build on Appveyor #496
net40
netstandard1.3
netstandard1.5

~~My suggestion is to get #485 done first~~ (Update: ✔️), then I'll rebase this PR and complete the removal of all obsolete targets.

src/Castle.Core/Components.DictionaryAdapter/Xml/Internal/Cursors/XPath/XPathCompiler.cs

stakx · 2020-05-13T21:40:57Z

src/Castle.Services.Logging.NLogIntegration/Castle.Services.Logging.NLogIntegration.csproj

@@ -3,7 +3,7 @@
 	<Import Project="..\..\buildscripts\common.props"></Import>

 	<PropertyGroup>
-		<TargetFrameworks>net45;netstandard1.5</TargetFrameworks>
+		<TargetFrameworks>net45;netstandard1.5;netstandard2.0;netstandard2.1</TargetFrameworks>


I suppose adding just netstandard2.0 would be sufficient. I've included netstandard2.1 mainly for consistency with the main library—I expect it'll be less confusing in the future to see the same list of target frameworks everywhere. (Same goes for the other logging integration projects.)

stakx · 2020-05-13T21:52:06Z

@jonorossi, this should be good to go.

I've made these changes one target at a time, which is why the commit history may seem a little repetitive. It should make reviewing a little easier, though.

There are some more things that could be done:

Checking whether we can simplify the Type vs. TypeInfo API mess. IIRC, .NET Standard 2.x has gravitated back towards the traditional Reflection APIs, so it might be possible to remove a few .GetTypeInfo()s here and there. This didn't seem too urgent to me, so I might do this in a separate PR. (Update: ✔️)
It might be good to shift PublicApiGenerator execution to the .NET Core build, so that we get as much work done on the cross-platform build as possible. This is more of a CI concern, it might be more appropriate to do this once we rewrite the CI scripts (for Move from AppVeyor to GitHub Actions #497).

What do you think?

jonorossi

🎉 Great work, that is a good clean up. Luckily things have moved on a lot since .NET Standard 1.x.

jonorossi · 2020-05-16T07:43:41Z

buildscripts/CommonAssemblyInfo.cs

-#if FEATURE_SECURITY_PERMISSIONS && DOTNET40
+#if FEATURE_SECURITY_PERMISSIONS
 [assembly: SecurityRules(SecurityRuleSet.Level2)]
 #endif


If it isn't already, removing all the security transparency junk should be on the list for this major release.

But leave it for another pull request, this one is already big 😉

I'll do that next.

jonorossi · 2020-05-16T07:49:52Z

src/Castle.Core.Tests.WeakNamed/DynamicProxy.Tests/BasicClassProxyTestCase.cs

@@ -29,42 +29,42 @@ public class BasicClassProxyTestCase : BasePEVerifyTestCase
 		public void ProxyForBaseTypeFromUnsignedAssembly()
 		{
 			Type t = typeof(Class);
-			Assert.False(StrongNameUtil.IsAssemblySigned(t.GetTypeInfo().Assembly));


I do always wonder what .NET would look like today if Microsoft had iterated on the .NET Framework rather than starting from scratch (i.e. ProjectK, DNX, .NET Core). .NET Core 3.1 BCL and tooling is very similar to .NET Framework and the migration wouldn't have been so painful for so long. Could have just had a self-contained .NET Framework 5, 6 or 7 with breaking changes between releases.

Glad to see GetTypeInfo calls gone, from a purist viewpoint they made sense, but they definitely get in the way.

Yeah, now that they have announced .NET 5 one cannot help but wonder. Feels like we've come full circle, but I suppose it's been a learning experience for the people behind the runtime, too. And had we followed a more direct path, me might have never gotten the runtime open-sourced, nor .NET Standard; both are great achievements.

The NuGet story, OTOH, has been one huge disaster. 😄

jonorossi · 2020-05-16T08:10:30Z

README.md

 * `FEATURE_APPDOMAIN` - enables support for features that make use of an AppDomain in the host.
 * `FEATURE_ASSEMBLYBUILDER_SAVE` - enabled support for saving the dynamically generated proxy assembly.
-* `FEATURE_BINDINGLIST` - enables support features that make use of System.ComponentModel.BindingList.
-* `FEATURE_DICTIONARYADAPTER_XML` - enable DictionaryAdapter Xml features.
-* `FEATURE_CUSTOMMODIFIERS` - enables reading and emitting optional and required custom modifiers defined on parameters including return parameters. It seems like a defect in corefx not to expose these methods because they are still implemented.
 * `FEATURE_EVENTLOG` - provides a diagnostics logger using the Windows Event Log.
-* `FEATURE_GAC` - enables support for obtaining assemblies using an assembly long form name.
-* `FEATURE_GET_REFERENCED_ASSEMBLIES` - enables code that takes advantage of System.Reflection.Assembly.GetReferencedAssemblies().
-* `FEATURE_IDATAERRORINFO` - enables code that depends on System.ComponentModel.IDataErrorInfo.
-* `FEATURE_ISUPPORTINITIALIZE` - enables support for features that make use of System.ComponentModel.ISupportInitialize.
-* `FEATURE_LEGACY_REFLECTION_API` - provides a shim for .NET 4.0 that emulates the `TypeInfo` API available in .NET 4.5+ and .NET Core.
-* `FEATURE_LISTSORT` - enables support for features that make use of System.ComponentModel.ListSortDescription.
-* `FEATURE_NETCORE_REFLECTION_API` - provides shims to implement missing functionality in .NET Core that has no alternatives.
 * `FEATURE_REMOTING` - supports remoting on various types including inheriting from MarshalByRefObject.
 * `FEATURE_SECURITY_PERMISSIONS` - enables the use of CAS and Security[Critical|SafeCritical|Transparent].
 * `FEATURE_SERIALIZATION` - enables support for serialization of dynamic proxies and other types.
-* `FEATURE_SMTP` - provides the email sender abstraction and implementation.
 * `FEATURE_SYSTEM_CONFIGURATION` - enables features that use System.Configuration and the ConfigurationManager.
-* `FEATURE_TARGETEXCEPTION` - enabled catching a `TargetException`. `System.Reflection.TargetException` is implemented by .NET Core but not exposed by corefx.
 * `FEATURE_TEST_COM` - enables COM Interop tests.
-* `FEATURE_TEST_DATASET` - enables tests that involve `System.Data.DataSet`s.
 * `FEATURE_TEST_PEVERIFY` - enables verification of dynamic assemblies using PEVerify during tests. (Only defined on Windows builds since Windows is currently the only platform where PEVerify is available.)
-* `FEATURE_TEST_SERILOGINTEGRATION` - enables Serilog integration tests.


After this is merged we can talk through these, more definitely need to go.

Agreed. I'll open an issue for that.

jonorossi · 2020-05-16T08:14:30Z

It might be good to shift PublicApiGenerator execution to the .NET Core build, so that we get as much work done on the cross-platform build as possible. This is more of a CI concern, it might be more appropriate to do this once we rewrite the CI scripts

Makes sense, definitely think it'll be easier when the build scripts are cleaned up.

Otherwise we cannot remove the last remaining `netstandard1.x` target without losing .NET Core support.

Those are now defined for all target frameworks and therefore no longer needed: * `FEATURE_BINDINGLIST` * `FEATURE_DICTIONARYADAPTER_XML` * `FEATURE_GAC` * `FEATURE_GET_REFERENCED_ASSEMBLIES` * `FEATURE_IDATAERRORINFO` * `FEATURE_ISUPPORTINITIALIZE` * `FEATURE_LISTSORT` * `FEATURE_NETCORE_REFLECTION_API` * `FEATURE_NETSTANDARD2_COMPATIBILITY` along w/ `.AsType()` calls * `FEATURE_SMTP` * `FEATURE_TARGETEXCEPTION` * `FEATURE_TEST_DATASET` * `FEATURE_TEST_SERILOGINTEGRATION` And further: * some usages of `DOTNET45` where it guards `XmlConvert` (which is now also available on all targets).

stakx marked this pull request as draft May 1, 2020 18:09

This was referenced May 1, 2020

add targets netstandard2.0 and netstandard2.1 #485

Merged

Fix broken Linux CI build on Appveyor #496

Merged

stakx force-pushed the remove-targets branch from 6ba96f4 to d4164cd Compare May 6, 2020 14:58

stakx commented May 6, 2020

View reviewed changes

src/Castle.Core/Components.DictionaryAdapter/Xml/Internal/Cursors/XPath/XPathCompiler.cs Outdated Show resolved Hide resolved

stakx changed the title ~~Remove framework targets net35, net40, netstandard1.3, netstandard1.5~~ Remove framework targets net40, netstandard1.3, netstandard1.5 May 6, 2020

stakx added this to the v5.0.0 milestone May 6, 2020

stakx force-pushed the remove-targets branch 4 times, most recently from 66a0f5e to c328a6b Compare May 13, 2020 21:35

stakx commented May 13, 2020

View reviewed changes

stakx marked this pull request as ready for review May 13, 2020 21:45

stakx requested a review from jonorossi May 13, 2020 21:52

jonorossi approved these changes May 16, 2020

View reviewed changes

stakx added 13 commits May 16, 2020 10:50

Fix redundant or incorrect uses of DOTNET40

5238a1d

Remove net40 target framework

5175121

Update ref/ contracts

6d1d3c1

Remove netstandard1.3 target framework

371cbe5

Remove redundant FEATURE_CUSTOMMODIFIERS symbol

29a2f0d

Update ref/ contracts

599aaf2

Add netstandard2.x targets to logging integration projects

d1449ce

Otherwise we cannot remove the last remaining `netstandard1.x` target without losing .NET Core support.

Update ref/ contracts

ebeddbc

Remove netstandard1.5 target

82763e1

Update ref/ contracts

5c8c352

Update version numbers in documentation

de1e802

Remove unnecessary package & assembly refs

98f3740

stakx added 3 commits May 16, 2020 10:50

We can run XML, Serilog, and NLog tests on .NET Core 2+

d1857bd

More precise column titles in compilation symbol table

2c8c00f

Remove those pesky .GetTypeInfo()

ebd80f8

stakx force-pushed the remove-targets branch from f22a484 to ebd80f8 Compare May 16, 2020 08:51

stakx merged commit 77e76b3 into castleproject:master May 16, 2020

stakx deleted the remove-targets branch May 16, 2020 08:56

stakx mentioned this pull request May 16, 2020

Remove obsolete or redundant conditional compilation symbols #501

Closed

stakx mentioned this pull request Jun 6, 2020

Make most of DynamicProxy's internals internal #505

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove framework targets `net40`, `netstandard1.3`, `netstandard1.5` #495

Remove framework targets `net40`, `netstandard1.3`, `netstandard1.5` #495

stakx commented May 1, 2020 •

edited

stakx May 13, 2020

stakx commented May 13, 2020 •

edited

jonorossi left a comment

jonorossi May 16, 2020

stakx May 16, 2020

jonorossi May 16, 2020

stakx May 16, 2020

jonorossi May 16, 2020

stakx May 16, 2020

jonorossi commented May 16, 2020

Remove framework targets net40, netstandard1.3, netstandard1.5 #495

Remove framework targets net40, netstandard1.3, netstandard1.5 #495

Conversation

stakx commented May 1, 2020 • edited

stakx May 13, 2020

Choose a reason for hiding this comment

stakx commented May 13, 2020 • edited

jonorossi left a comment

Choose a reason for hiding this comment

jonorossi May 16, 2020

Choose a reason for hiding this comment

stakx May 16, 2020

Choose a reason for hiding this comment

jonorossi May 16, 2020

Choose a reason for hiding this comment

stakx May 16, 2020

Choose a reason for hiding this comment

jonorossi May 16, 2020

Choose a reason for hiding this comment

stakx May 16, 2020

Choose a reason for hiding this comment

jonorossi commented May 16, 2020

Remove framework targets `net40`, `netstandard1.3`, `netstandard1.5` #495

Remove framework targets `net40`, `netstandard1.3`, `netstandard1.5` #495

stakx commented May 1, 2020 •

edited

stakx commented May 13, 2020 •

edited