Update MSBUILD nuget packages

ericstj commented 8 years ago

The MSBuild nuget packages are using deprecated nuget monikers, a very old version of CoreFx packages (and in some cases mis-matched versions), and are missing the version spec for dependencies on their own packages. This all needs to be fixed up. I can help you understand how to do this, or even contribute the changes if you point me to where you build these packages.

/cc @dsplaisted

dsplaisted commented 8 years ago

We have #528 for the versioning of dependencies between the packages. Is there a need to update the dependencies for the .NET Core packages before RC2 is released?

The packages are built out of the xplat branch. Each project has a project.json file declaring its dependencies. We use the Nuspec reference generator to update the dependencies in the nuspecs.

Updating the monikers will also require changes in some of our targets/props files (e.g. here).

The project.json in targets/runtimeDependencies will also need updating. It is what we use to deploy MSBuild along with its dependencies for testing. It has to duplicate the dependencies from all the individual project.jsons, so separately it would be nice to come up with a better solution for that.

rainersigwald commented 8 years ago

We want to update the CoreFx package references for #566, too--last time I tried I got some compilation errors from the then-current CoreFx daily build that I didn't dig into.

ericstj commented 8 years ago

I can help you work through any errors you're seeing. Now would be a good time to update to RC2 packages. @dsplaisted nothing forcing a deadline here, I just noticed this when looking at warnings with @KevinRansom.

rainersigwald commented 8 years ago

@ericstj here's my work-in-progress branch: https://github.com/rainersigwald/msbuild/tree/update-corefx-references

The first error I see is

Logging\LoggerDescription.cs(240,33): error CS0246: The type or namespace name 'TypeFilter' could not be found (are you missing a using directive or an assembly reference?) [D:\msbuild\src\XMakeBuildEngine\Microsoft.Build.csproj]

I'm also getting

D:\msbuild\src\Shared\NativeMethodsShared.cs(571,85): error CS1061: 'Assembly' does not contain a definition for 'Location' and no extension method 'Location
' accepting a first argument of type 'Assembly' could be found (are you missing a using directive or an assembly reference?) [D:\msbuild\src\Utilities\Micros
oft.Build.Utilities.csproj]
D:\msbuild\src\Shared\FileUtilities.cs(58,72): error CS0117: 'Environment' does not contain a definition for 'GetCommandLineArgs' [D:\msbuild\src\Utilities\M
icrosoft.Build.Utilities.csproj]

That project.json declares a dependency on System.Runtime.Extensions which seems like it should satisfy at least Environment.GetCommandLineArgs.

ericstj commented 8 years ago

All of these are only available in NETStandard1.5, you're targeting NETStandard1.3. These Types/members were new based on what was previously shipped for UWP & pinned by the toolchain so we had put them in a new generation.

dsplaisted commented 8 years ago

@ericstj Targeting .NETStandard 1.5 would mean you wouldn't be able to compile tasks against the .NET Core versions of our assemblies and run using full framework MSBuild on .NET 4.6. To fix that, we'd need to create separate reference assemblies for .NETStandard 1.3.

rainersigwald commented 8 years ago

Not being able to do universal tasks is a problem. I tried going to .NETStandard1.5 (https://github.com/rainersigwald/msbuild/tree/update-corefx-references again), and it almost works, but the runtime deployment fails. If I leave the runtime project.json as is, it fails complaining about not including netstandard1.5, ditto if I change the current netstandard1.3 to 1.5. If I change the runtime from dnxcore50 to netstandard1.5, I get a resolution error

D:\msbuild\targets\DeployDependencies.proj(103,5): error : Your project.json doesn't list 'win7-x64' as a targeted runtime. You should add '"win7-x64": { }' inside your "runtimes" section in your project.json, and then re-run NuGet restore.

That runtime is in the project.json, but it doesn't appear in the output lock file. Updating our dotnet CLI version didn't seem to fix that.

cdmihai commented 8 years ago

@ericstj what version of corefx do you recomment we update to? I also had a stab at bumping up our corefx dependencies to what seemed the latest (rc3-24105-00) but restore fails because we have a dependency on "xunit.netcore.extensions": "1.0.0-prerelease-00405-03" which seems to depend on rc2, thus dragging everything down from rc3 to rc2 :(

My branch is here: https://github.com/cdmihai/msbuild/tree/nugetFixes Has similar changes to Rainer's except for newer corefx packages and keeping msbuild to 1.3

Errors in C:\projects\msbuild\targets\runtimeDependencies\project.json
      System.Runtime.InteropServices.RuntimeInformation 4.0.0-rc2-23923 provides a compile-time reference assembly for System.Runtime.InteropServices.RuntimeInformation on .NETFramework,Version=v4.6, but there is no run-time assembly compatible with osx.10.10-x64.
      One or more packages are incompatible with .NETFramework,Version=v4.6 (osx.10.10-x64).
      System.Runtime.InteropServices.RuntimeInformation 4.0.0-rc2-23923 provides a compile-time reference assembly for System.Runtime.InteropServices.RuntimeInformation on .NETFramework,Version=v4.6, but there is no run-time assembly compatible with ubuntu.14.04-x64.
      One or more packages are incompatible with .NETFramework,Version=v4.6 (ubuntu.14.04-x64).
      System.Runtime.InteropServices.RuntimeInformation 4.0.0-rc2-23923 provides a compile-time reference assembly for System.Runtime.InteropServices.RuntimeInformation on .NETFramework,Version=v4.6, but there is no run-time assembly compatible with win7-x64.
      One or more packages are incompatible with .NETFramework,Version=v4.6 (win7-x64).
      System.Runtime.InteropServices.RuntimeInformation 4.0.0-rc2-23923 provides a compile-time reference assembly for System.Runtime.InteropServices.RuntimeInformation on .NETFramework,Version=v4.6, but there is no run-time assembly compatible with win7-x86.
      One or more packages are incompatible with .NETFramework,Version=v4.6 (win7-x86).
      System.Security.Cryptography.X509Certificates 4.1.0-rc2-23908 provides a compile-time reference assembly for System.Security.Cryptography.X509Certificates on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with osx.10.10-x64.
      System.Linq 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Linq on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with osx.10.10-x64.
      System.Security.Cryptography.Algorithms 4.2.0-rc3-24105-00 provides a compile-time reference assembly for System.Security.Cryptography.Algorithms on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with osx.10.10-x64.
      System.Linq.Expressions 4.1.0-rc3-24018-00 provides a compile-time reference assembly for System.Linq.Expressions on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with osx.10.10-x64.
      System.ComponentModel.Annotations 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.ComponentModel.Annotations on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with osx.10.10-x64.
      System.Net.Http 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Net.Http on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with osx.10.10-x64.
      System.Text.RegularExpressions 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Text.RegularExpressions on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with osx.10.10-x64.
      One or more packages are incompatible with .NETStandard,Version=v1.3 (osx.10.10-x64).
      System.Security.Cryptography.X509Certificates 4.1.0-rc2-23908 provides a compile-time reference assembly for System.Security.Cryptography.X509Certificates on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with ubuntu.14.04-x64.
      System.Linq 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Linq on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with ubuntu.14.04-x64.
      System.Security.Cryptography.Algorithms 4.2.0-rc3-24105-00 provides a compile-time reference assembly for System.Security.Cryptography.Algorithms on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with ubuntu.14.04-x64.
      System.Linq.Expressions 4.1.0-rc3-24018-00 provides a compile-time reference assembly for System.Linq.Expressions on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with ubuntu.14.04-x64.
      System.ComponentModel.Annotations 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.ComponentModel.Annotations on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with ubuntu.14.04-x64.
      System.Net.Http 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Net.Http on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with ubuntu.14.04-x64.
      System.Text.RegularExpressions 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Text.RegularExpressions on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with ubuntu.14.04-x64.
      One or more packages are incompatible with .NETStandard,Version=v1.3 (ubuntu.14.04-x64).
      System.Security.Cryptography.X509Certificates 4.1.0-rc2-23908 provides a compile-time reference assembly for System.Security.Cryptography.X509Certificates on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x64.
      System.Linq 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Linq on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x64.
      System.Security.Cryptography.Algorithms 4.2.0-rc3-24105-00 provides a compile-time reference assembly for System.Security.Cryptography.Algorithms on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x64.
      System.Linq.Expressions 4.1.0-rc3-24018-00 provides a compile-time reference assembly for System.Linq.Expressions on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x64.
      System.ComponentModel.Annotations 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.ComponentModel.Annotations on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x64.
      System.Text.RegularExpressions 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Text.RegularExpressions on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x64.
      One or more packages are incompatible with .NETStandard,Version=v1.3 (win7-x64).
      System.Security.Cryptography.X509Certificates 4.1.0-rc2-23908 provides a compile-time reference assembly for System.Security.Cryptography.X509Certificates on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x86.
      System.Linq 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Linq on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x86.
      System.Security.Cryptography.Algorithms 4.2.0-rc3-24105-00 provides a compile-time reference assembly for System.Security.Cryptography.Algorithms on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x86.
      System.Linq.Expressions 4.1.0-rc3-24018-00 provides a compile-time reference assembly for System.Linq.Expressions on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x86.
      System.ComponentModel.Annotations 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.ComponentModel.Annotations on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x86.
      System.Text.RegularExpressions 4.1.0-rc3-24105-00 provides a compile-time reference assembly for System.Text.RegularExpressions on .NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x86.
      One or more packages are incompatible with .NETStandard,Version=v1.3 (win7-x86).

In the meantime I will look into whether we can remove our dependencies to TypeFilter, Assembly.Location and Environment.GetCommandLineArgs. Worst case I will copy paste over the code.

ericstj commented 8 years ago

It'd be good for you guys to update at least to RC2 latest.

"xunit.netcore.extensions": "1.0.0-prerelease-00405-03" which seems to depend on rc2, thus dragging everything down from rc3 to rc2 :(

That doesn't happen. You may get a mix, but the project referencing is the only thing that can downgrade. If two versions are seen the higher will win so long as it isn't the parent (project) who is requesting the lower version.

You have two flavors of errors here:

.NETStandard,Version=v1.3, but there is no run-time assembly compatible with win7-x86.

.NETStandard isn't a run-able TFM, it shouldn't be resolved with Runtimes listed.

NETFramework,Version=v4.6, but there is no run-time assembly compatible with ubuntu.14.04-x64.

Desktop on Ubuntu? The RC3 version of this package (RuntimeInformation) wouldn't have this issue due to the refactoring I did to always make desktop resolve with no RID, but resolving desktop for unix is likely an error in the project.

I'll take a look at your branch tomorrow and see if I can help you update it.

cdmihai commented 8 years ago

Hmm, this might get tricky.

runtimeDependencies/project.json is not used at compile time. It is used to deploy all of MSBuild's dependencies into the deployment and testing outputs so that we can get a copy pastable msbuild or a testable msbuild. This happens in DeployDependencies.proj

The runtime project.json contains:

MSBuild's dependencies. These are duplicated from the multiple project.json files each msbuild project has.
test only dependencies. These are required to get xunit and such so we can run msbuild's tests and, on full framework, to have tests appear in VS.

We want to deploy these dependencies per each OS, hence the runtimes section for the runtime project.json. What we want to express is not "give me a runnable output for this OS" but "blindly paste in OS specific libraries from this dependency graph that nuget resolved". Is there a way to tell Nuget to not make checks for "is this target framework runnable"?

Regarding the issue of resolving .net46 for !windows, I see the issue. Theoretically, deploying .net46 for !windows does not happen since we tell the PrereleaseResolveNugetPackageAssets what runtime to target (https://github.com/Microsoft/msbuild/blob/xplat/targets/DeployDependencies.proj#L106). So the .net46/osx rid never comes up during runtime. However, Nuget does not know that. Do you suggest we split the runtime project.json in two? A windows version, and a !windows version?

Or, does project.json support qualifing runtimes per target framework? something like:

{
  "frameworks": {
    "net46": {
      "runtimes": {
        "win7-x86": {

        },
        "win7-x64": {

        }
      }
    },
    "dnxcore50": {
      "runtimes": {
        "win7-x86": {

        },
        "win7-x64": {

        },
        "osx.10.10-x64": {

        },
        "ubuntu.14.04-x64": {

        }
      }
    }
  }
}

jaredpar commented 8 years ago

@ericstj

All of these are only available in NETStandard1.5, you're targeting NETStandard1.3.

What does it take to move them back? Not having a portable MSBuild reference assembly that works on 4.6 and NetCore is becoming a blocker for the division. Asking MSbuild to move to a desktop version that we cannot support in the downstream tools doesn't seem like an actionable decision.

ericstj commented 8 years ago

@cdmihai, no explicit TFM/RID pairs, though it is a feature request. Workaround is to split the project.json into two, each with its own set of RIDs.

@jaredpar I agree. We need to have MSBuild's assemblies targeting at least NS1.3 (ideally even NS1.0) so that folks can build portable tasks. I actually think it is doing so today:https://github.com/Microsoft/msbuild/blob/xplat/src/XMakeTasks/project.json#L18

cdmihai commented 8 years ago

To have the discussion cleaner, I created a separate issue for fixing the msbuild runtime project.json to target multiple runtimes: #609

Let's keep this thread for discussing how to bump up the corefx versions to the latest and still have MSBuild target .netstandard1.3.

@ericstj Currently MSBuild is indeed targetting .netstandard1.3, as it should be. However, we cannot bump our corefx dependencies due to members / types that have been removed (Assembly.Location, Environment.GetCommandLineArgs, TypeFilter)

Options:

Use alternative APIs (can you please tell me what they are)
Use reflection for .Location and .GetCommandLineArgs and copy paste in the missing types (TypeFilter)

cdmihai commented 8 years ago

Seems like I may be able to replace TypeFilter with Func<Type, object, bool>. The MSBuild codebase appears to be using it directly and not passing it from / into the libraries: https://github.com/cdmihai/msbuild/commit/d6fd9a46590dbe883ee8346640f184f3fa8d5341

cdmihai commented 8 years ago

Also removed Environment.GeCommandLineArgs. It was used to detect if the execution is within a test. Since there are many other reasonable fallbacks after it, the loss in precision does not seem that bad: https://github.com/cdmihai/msbuild/commit/6b8c549ca77ad211adb2d55f293f29a3eeafa920

Tests still pass after this, so it seems OK.

cdmihai commented 8 years ago

Only missing member left is Assembly.Location. @ericstj, is there a workaround for getting this without reflection?

ericstj commented 8 years ago

Using reflection is not a good plan since some run times won't have it (eg: NETNative). What sort of location do you need? Location of a task DLL? Location of MSBuild.exe?

ericstj commented 8 years ago

Looks like you are trying to find the location of the framework. As far as I can tell all the upstack code of this is assuming desktop-like layout of components in the framework directory (facades, paths to other desktop frameworks, etc). I wouldn't expect any of this to show up in your core implementation of MSBuild. Can you just leave this unset or default it to AppContext.BaseDirectory?

jaredpar commented 8 years ago

Roslyn hit the same issue around Assembly.Location. Our solution was to use reflection to work around the problem. True it won't work on .NETNative but ... is MSBuild really intended to work there? Should we hold up having a more vibrant ecosystem on .NETNative support? I'd vote no.

jaredpar commented 8 years ago

Have we considered adding back Assembly.Location just throwing a NotSupportedException in the NETNative case? This has been a blocking issue on a number of products moving to NetCore. Most end up choosing to use reflection to work around the problem which is a bad solution.

ericstj commented 8 years ago

@jaredpar if you'd like to file an issue on that we'll have a discussion in CoreFx. Short answer: that type impl lives in the shared library which cannot change. We can't OOB properties since we don't have extension properties. We could do something elaborate with IL-transforms but I don't want to have that discussion here. In this case the whole code-path is legacy and ought to be trimmed from the core implementation.

jaredpar commented 8 years ago

In this case the whole code-path is legacy and ought to be trimmed from the core implementation.

This is a constant blocker for deveolopers moving to .NET Core. It's blocked by a single corner case scenario. It seems strange to declare it Legacy and ask people to fundamental change their apps to move here.

ericstj commented 8 years ago

@jaredpar as I mentioned we're interested in bringing more of those things back (and indeed we did in this case, just not for PCLs that target NS1.3) but you aren't going to get anywhere by venting in this issue. Please open an issue in CoreFx and articulate your goal, for example: "Make System.Reflection 4.1 surface area available in NETStandard1.3".

I'll use a different phrase here rather than legacy: completely broken in the context of a core app. This code was trying to locate a bunch of things by assuming they'd be next to mscorlib:

        /// <summary>
        /// Gets the currently running framework path
        /// </summary>
        internal static string FrameworkCurrentPath
        {
            get
            {
                if (s_frameworkCurrentPath == null)
                {
                    s_frameworkCurrentPath =
                        Path.GetDirectoryName(typeof(string).GetTypeInfo().Assembly.Location)
                        ?? string.Empty;
                }

                return s_frameworkCurrentPath;
            }
        }

That's not going to work in Core, so even if the API were available it'd be wrong to use it in this way (and likely lead to a lot of wasted FileSystem interactions). Instead it might be easier to just have it return a default value for Core. Now if you're trying to get the same IL working in both Desktop and Core (I don't think that's the case here). You could use reflection and just make sure to have this return the default value if you couldn't get at the property.

cdmihai commented 8 years ago

@ericstj Assembly.Location is used in other places as well, not just in FileUtilities. For example:

Multi process node communication: The DLL location is part of MSBuild's serialization format, node handshake, etc
Multi process node instantiation: The DLL location is used to find out where to spawn new msbuild nodes from
providing good logs for tasks loaded from arbitrary DLLs (which task executed, which task crashed)

Changing those is a bit risky or not a good user experience. To get us unblocked, for now, we'll look into using reflection.

cdmihai commented 8 years ago

For the discussion on bringing back Assembly.Location, or an alternative for retrieving the path an assembly got loaded from I opened this issue: https://github.com/dotnet/corefx/issues/8398

cdmihai commented 8 years ago

Alright, Assembly.Location has been dealt with: https://github.com/cdmihai/msbuild/commit/a6a434d0ce5d81e9164289dc5b093aa761b7cfd0 https://github.com/cdmihai/msbuild/commit/d790f5379be53b2875130fd8f2adcd8601ed7d40

All the missing API problems were resolved (worked around). The only thing that still blocks msbuild updating to the latest corefx is issue #609.

cdmihai commented 8 years ago

With @ericstj's help, I got to deploy MSBuild's runtime dependencies and run tests against the new bumped up corefx versions.

There's 3 failing tests due to corefx changes:

test1: PropertyFunctionStaticMethodFileAttributes fails because Type.GetType("System.IO.File") returns null
test2: PropertyFunctionStaticMethodEnumArgument fails because System.Environment.GetFolderPath got removed
test3: NormalizePathBadGlobalroot fails because Path.GetFullPath(@"\\?\globalroot") is expected to throw ArgumentException("Paths that begin with \\\\?\\GlobalRoot are internal to the kernel and should not be opened by managed applications.") but after the corefx version bump up it does not throw that exception anymore.

@ericstj, what should we do about these? :) Test3 may sound like a bug in Path. Test2 implies that msbuild build scripts are not portable anymore, since some property functions may break on .net core. Test1 is weird.

ericstj commented 8 years ago

Test1 and Test2 are failing because those members were cleaned up out of mscorlib. In the case of File this was moved to System.IO.FileSystem In the case of GetFolderPath, this is currently not available. There is not a good cross-plat representation of this API.

For property functions we might want to have some "not supported" behavior that lets folks know that these aren't going to work on Unix.

For test3 I recall @JeremyKuhne making some changes around this for long path. You may want to sync with him about understanding if this was a bug or intentional change in behavior.

cdmihai commented 8 years ago

@ericstj Until Corefx makes transparent the fact that types are moving out from mscorlib into other dlls, should we maintain a map from white listed types to their new dlls? I guess that's the only way to load them for now, right?

ericstj commented 8 years ago

/cc @weshaggard you shouldn't rely on mscorlib surface at all. Mscorlib isn't public surface for NETCore which is why types are moved/removed on Netcore. We are working on more mscorlib compat but I wouldn't reccomend depending on that since it won't be ready for some time. I'd reccomend using a table to lookup assembly and probing that first before falling back to GetType without an assembly. That's what vs is doing for capability probes.

JeremyKuhne commented 8 years ago

test3 was an intentional change in behavior. \\?\GlobalRoot\ wasn't understood and it was blocked unnecessarily and incorrectly. \\.\GlobalRoot\ is the exact same path, for example, as is \\?\Global\GlobalRoot\. GlobalRoot is a link to the root of the Object Manager namespace- it doesn't give you access to anything you don't have rights to through "normal" paths. The MSBuild test was trying to validate that it matched Path.GetFullPath's original behavior.

I recommend that the code use Path.GetFullPath directly now. GetFullPath doesn't have the same perf issues as it had originally and it also supports long paths, which are currently blocked by MSBuild.

analogrelay commented 8 years ago

... and are missing the version spec for dependencies on their own packages

Ran in to this as well. I can work around it by manually adding top-level dependencies on the things MSBuild depends on, but it's unfortunate that I can't use transitive dependencies here.

cdmihai commented 8 years ago

@anurse Issue #528 tracks that. It is currently blocked by build tools accepting this PR: https://github.com/dotnet/buildtools/pull/763 As soon as that PR goes in, we can enable putting p2p versions in msbuild nuspecs

cdmihai commented 8 years ago

I'll close this issue since the only item not yet done is tracked by #528

dotnet / msbuild

Update MSBUILD nuget packages #587