.NET Core and VS2019 Compatibility #7

mnadareski · 2020-02-19T18:21:16Z

This PR does a number of things in order to bring forward compatibility to libmspack4n:

libmspack includes have been updated to conform to the new standard, namely that local includes should not use include <lib> and instead use include "lib". This is mostly cosmetic, but it also allows modern compilers to deal with the code
Along with the above, the libmspack project has been added to the solution, which allows it to be built in tandem with the other C# projects. This has the added benefit of allowing for enforcing dependencies in the C# projects
Updated csproj files to conform to the new standard that allows for .NET Core and .NET Standard projects. Some boilerplate such as file alignment was left out intentionally since it's not necessary for the new format
Added targets for .NET 4.6.1, .NET 4.6.2, .NET 4.7.2, .NET Core 2.1, and .NET Core 3.0. Removed .NET 4.0 target due to forward compatibility issues.
Added guards for LessIO so that the .NET Core 2.1 and 3.0 builds do not attempt to use LessIO until that is updated (I may take a stab at that myself if this PR goes over well)
Fixed some minor testing issues due to the csproj changes. I was not able to figure out why the second one failed (as per the existing Skip text), but I gave it my best and attempted to update the passing test to be more inline with XUnit expectations, including adding basic Assert statements. It's not perfect and I honestly don't know enough about the code to make any additional tests, but it's something.

This is a large change due to the sheer number of files and I can attempt to reduce the footprint if that is requested. A good number of these changes, especially in libmspack, were required by VS2019 when adding the vcxproj.

This fixes #6

mnadareski · 2020-02-19T18:23:22Z

Just noticed that AppVeyor is failing on this. I think it's out of the compatibility with the new csproj format. This new format (and support for .NET Core) was introduced in either VS2017 or VS2019, I forget which at this point.

mnadareski · 2020-02-19T18:37:44Z

Also of note, I can add as many .NET 4.5 and above targets as requested as a part of this, especially for the broadest direct compatibility

mnadareski · 2020-02-19T23:23:38Z

Okay, I apologize for the incredibly large amount of commits that went into the AppVeyor cleanup. I had to go through multiple layers of indirection in order to figure out what the current best way of approaching most of the things. This did remove a lot of now-superfluous files and made everything much more automated. I don't blame you if you want to just try to cherry-pick some of the changes I made to fit your current flow. If this ends up being accepted, I'd strongly suggest Squash and Merge, just due to the sheer amount of "bad" commits above.

activescott · 2020-02-24T06:38:37Z

Sorry for the delay and thanks for the PR! I had a busy weekend and tonight discovered my windows box is foobared right now but I’ll get it up and running and give this a close look before merging. Based on what you describe and the fact that the tests appear to be passing I don’t expect any issues. Thanks again!

activescott

Please put back the .bat files. I'm not interested in loading Visual Studio for any project, especially C++ projects. So I'd like to make sure we can always immediately jump in and build/compile it with a bat file and test it with a .bat file.

Why delete AssemblyInfo.cs? How is the assembly attributes set without it (again in a .bat/CI build)?

Please also pugt back the nuspec and nuget publishing. That stuff appears to be broken.

activescott · 2020-02-27T15:46:31Z

appveyor.yml


 #---------------------------------#
 #      build configuration        #
 #---------------------------------#
-build_script:
- - cmd: 'msbuild .\.build\libmspack4n.msbuild /p:TheVersion="%APPVEYOR_BUILD_VERSION%" /logger:"C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll"'


So how are you setting the build version? Maybe I'm missing where you added it back, but definitely we cannot just not set the version during a build.

activescott · 2020-02-27T15:48:13Z

appveyor.yml

-#---------------------------------#
-#       tests configuration       #
-#---------------------------------#
-test:


Have to have tests in place. Removing those is definitely not an option.

activescott · 2020-02-27T15:51:45Z

libmspack4n/MSCabinet.cs

@@ -35,8 +41,13 @@ public sealed class MSCabinet : IDisposable

 		public MSCabinet(string cabinetFilename)
 		{
-            cabinetFilename = new Path(cabinetFilename).WithWin32LongPathPrefix();


Did LessIO not build under .net core?

As I mentioned somewhere, else if you have reliable alternatives in .NET CORE for long file names and anything else LessIO is being used for I'm fine with removing the dependency on LessIO. I just want to make sure that LessMSI's tests that work with long file names and CABs continue to work.

activescott · 2020-02-27T15:53:36Z

libmspack4n/MSCompressedFile.cs

+			//TODO: Delete the file if it exists. If there are any issues ovewriting the dest file (e.g. it's readonly) MSPACK gives essentialy no error information.
+
+			#if NET_CORE
+			string longDestinationFilename = $"\\\\?\\{destinationFilename}";


If you're confident that this will work for those long file names and the tests pass for this and in LessMSI's usage of this library I'm fine with removing LessIO entirely from this particular assembly.

In fact, I'd prefer to remove it entirely rather than having #IF... Again assuming it works with long file names in all scenarios (some of which are tested in the LessMSI library)

activescott · 2020-02-27T16:00:11Z

libmspack4ntest/Basic.cs

+			Assert.False(errors);
+
+			Assert.True(Directory.Exists(outDir));
+
+			var outputFiles = Directory.GetFiles(outDir, "*", SearchOption.AllDirectories);
+			Assert.Equal(fileCount, outputFiles.Count());
+
+			Directory.Delete(outDir, true);


Super minor, but I'd prefer to eliminate the additional spacing in these lines.

activescott · 2020-02-27T16:02:42Z

libmspack4ntest/Basic.cs

 using System.IO;
-using System.Reflection;
+using System.Linq;


I know this is just tests, but don't see where we are we taking a dependency on Linq?

mnadareski · 2020-02-27T22:31:17Z

I will be getting to the requested changes when I can. I ran into some hardware issues which means having to re-setup my development environment. I will try to do so in a reasonable timeframe.

activescott · 2020-03-01T01:37:43Z

Totally understand. I appreciate you taking the time. I feel bad about requesting so many changes, but I just feel like some of these things are critical to making it so I can keep this project maintainable. Thanks for understanding!

activescott · 2020-10-15T06:02:51Z

I'm going to close this since it is stale. If you want to re-open please do so or ping me and we'll get it going again.

.NET Core and VS2019 Compatibility

fcb9e80

mnadareski requested a review from activescott February 19, 2020 18:24

mnadareski mentioned this pull request Feb 19, 2020

Fix .NET Core 3.0 build SabreTools/BinaryObjectScanner#7

Closed

mnadareski added 23 commits February 19, 2020 11:23

Try to address AppVeyor issues

d593224

Forgot about the multiple solution issue

3adf932

Nuget restore, of course

83bbd58

VCVars

24e687e

2019 not 2017

8f747c3

Try to fix the envars

90ca21c

Wish there was an appveyor tester

aea8440

Version

ccc7c6f

Slash

9f352e2

Move nuspec

f9ee9a4

Fix pathing

406b35b

Explicit platform and config?

9680896

Fix nuspec, manual nuget pack

e3d2059

Version var

f911f90

Try using built-in things

d50e4a5

Cleanup cleanup

2dc534e

Naming woes

8acd1ee

Fix tests, there was a reason that reference was there

23793d3

Let me try again

5c6ee93

Quote unquote

0d2d3aa

Disable automatic testing because AppVeyor is great

0f95d1c

One last thing

437170f

Alternate

e87166a

activescott requested changes Feb 27, 2020

View reviewed changes

activescott closed this Oct 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

.NET Core and VS2019 Compatibility #7

.NET Core and VS2019 Compatibility #7

mnadareski commented Feb 19, 2020 •

edited

Loading

mnadareski commented Feb 19, 2020

mnadareski commented Feb 19, 2020

mnadareski commented Feb 19, 2020

activescott commented Feb 24, 2020

activescott left a comment

activescott Feb 27, 2020

activescott Feb 27, 2020

activescott Feb 27, 2020

activescott Feb 27, 2020

activescott Feb 27, 2020

activescott Feb 27, 2020

activescott Feb 27, 2020

activescott Feb 27, 2020

mnadareski commented Feb 27, 2020

activescott commented Mar 1, 2020

activescott commented Oct 15, 2020

.NET Core and VS2019 Compatibility #7

.NET Core and VS2019 Compatibility #7

Conversation

mnadareski commented Feb 19, 2020 • edited Loading

mnadareski commented Feb 19, 2020

mnadareski commented Feb 19, 2020

mnadareski commented Feb 19, 2020

activescott commented Feb 24, 2020

activescott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnadareski commented Feb 27, 2020

activescott commented Mar 1, 2020

activescott commented Oct 15, 2020

mnadareski commented Feb 19, 2020 •

edited

Loading