Remove the magic "ReferenceAssembliesDirectory" #67
Conversation
…e of DependencyContext.CompileLibraries on Core. ikvmc in the default case should search for reference assemblies in it's own ref/ directory.
…lish output of ikvmc for each of the desired platforms. Test cases will attempt to launch published ikvmc.
…erently based on the platform.
…rences. Work in progress.
Locate Runtime directory and references in test differently for framework vs Core.
… against it. But this is only effective for Core. For Framework, it invokes it with -lib: and the Framework directory.
…ration. Resolver now has a single path, checking the universe for existing assemblies, then the search path, and finally comparing any potential matches based on version. Change error message for StubsAreDeprecated to simply stubs are invalid. Been long enough I'd imagine.
|
So, I've gone ahead and done some more stuff. After examining how Roslyn currently works, and the effects of the -nostdlib option, and -lib and -ref, and thinking through our future usages, I think it makes sense to deprecate the -nostdlib option, as well as the default SDK search logic. The situation is this. In the latest C# compilers (upon which ikvmc is modeled after) included with .NET Core, -nostdlib is somewhat of a no-op. This is because in Core, there is not necessarily any default SDK to use. The Core csc.exe from 6.0 could be used to compile 5.0 assemblies, or 3.1 assemblies, or even Framework assemblies. It's no longer safe for it to simply assume the user intends to compile targeting the version of the platform that csc.exe was ultimately built in. So, with this PR, simply running ikvmc, for any platform, will result in it being unable to find any references to build against. Including mscorlib or netstandard. Instead, the user has to specify them. The user has two ways of doing this: -reference (-r) or -lib. He can either reference the assemblies specifically using -reference, or provide a search path using -lib. The simplest use case for a user targeting a known Framework, would be to use the -lib option to point to the SDK he wants to use. If he's targeting Framework, he should specify -lib:C:\Windows\Microsoft.NET\version\Framework64. If he's targeting some .NET Core SDK, he can use -lib to point to the reference assembly folder of that SDK. And if he's invoking ikvmc programmatically from within another .NET application, he has use -lib to point to his own ref/ folder (PreserveCompilationContext). It's up to him where to point. But he has to be explicit about it. ikvmstub got the same treatment, for the same reasons. Thus, the tools packages no longer need to distribute a compilation context. There was a lot of very old stuff going on in ikvmc that I ended up removing as a part of this. First, all of the stuff regarding preloading the references path based on the Framework, gone. All of the stuff about using the Framework path as a fallback, gone. There's just the single -lib option. I stumbled onto some other legacy stuff: Apparently there used to be an old capability to compile a Stub to a DLL. This was disabled years ago, but references were sucked in from stub files to keep old systems going. We're far from that at this point, so that's removed. If you try to compile a stub, you just get a warning and it ignores the stub. ikvmstub was also using a totally different resolution logic called LegacyResolveReference. I removed this and unified it with the logic used by ikvmc. They should be identical in this regard. A few files got the auto format treatment while I was in them. Code was passing around arrays to append data to. With a copy ArrayCopy methods to copy the arrays and append data. I think they did this before generics existed! They wanted strong types for the list, and ArrayList wasn't strongly typed! Replaced this with List.Add(). There is now two new test projects related to ikvmc. One uses ikvmc as a reference, and call's its main method programmatically to convert a small dynamic jar into an assembly. The other invokes ikvmc.exe as an executable to do the same. These tests don't do much except verify there is no crashing error going on. Ideally they would actually validate the output of the assembly. But not letting a release slip by with completely broken tools is a step forward. There was a check that prevented ikvmc and ikvmstub from loading a mscorlib or netstandard that was of a higher version than the one they were built against. I do not think this was necessary. If a user wants to compile against an uplevel framework, let them. We should support our netcore3.1 tool compiling net5 assemblies. |
Generate stub for a local ref library, instead of for a different runtime library.
| } | ||
| return dir; | ||
| } | ||
| public static readonly string StdLibName = "netstandard"; |
There was a problem hiding this comment.
I thought that the mscorlib functionality was moved into an assembly named System.Private.CoreLib (or at least that seems to be the case since .NET 5.0). This seems to be backed up here: dotnet/roslyn#16211 (comment). Although, later they mention that it is
System.Runtimeat design time andSystem.Private.CoreLibat runtime, but the latter is an implementation detail
Why is netstandard being used here? Could this be an artifact from the early attempts at targeting netstandard2.0 and later netstandard2.1? And could it be the reason for #74?
I am not saying that this is wrong per se, but I am wondering if there is any documentation indicating netstandard is what we should be looking for.
There was a problem hiding this comment.
Because there are type forwards in it, and the code path doesn't differentiate between the static and dynamic compilers. It is not optimal.
There was a problem hiding this comment.
They mention here:
You have to pass in all assemblies that are involved in the compilation. Type forwarders are followed but you need to specify both the assembly that defines the forwarder and the assembly that the forwarder forwards to.
Is this being accounted for?
There was a problem hiding this comment.
If the user passes them in. The goal is to follow the same requirements as Roslyn. So, if the Roslyn guys say you need to do that, we should tell our users they need to do that.
There was a problem hiding this comment.
That said, this PR isn't a complete overhaul of the way assemblies are located and reflected on. It's a minor fix that removes the hard coded attempt at discovering a search path, and unifies two different discovery paths. The rest of the code is still doing what it did before: expecting a core library with the core types like Object, String, etc.
It would be better, in my view, if IKVM discovered these by actually searching the assemblies.
There was a problem hiding this comment.
That explains why #74 is failing (and why I am getting the same error when trying to convert log4j). However, since we are not actually shipping netstandard.dll with IKVM tools (it isn't in the .NET Core 3.1 reference assemblies directory), what is the procedure we should give users to provide a path to it for ikvmc?
On my system, I have several copies, but the advice here is to use C:\Program Files\dotnet\sdk\2.0.3\Microsoft\Microsoft.NET.Build.Extensions\net461\lib\netstandard.dll. It sounds like you need Visual Studio to be installed in order for this path to exist, though. And it is a little unsettling that it is in a net461 folder.
It looks like there are a couple of NuGet packages depending on whether we need netstandard2.0 or netstandard2.1. How do users discover which one to use? And what if there are dependencies that target netstandard1.x?
There was a problem hiding this comment.
Hmm. I had thought netstandard was around in every core sdk for compatibility.
There was a problem hiding this comment.
Looks like it is. But it is not in the ref/ directory, it is under C:\Program Files\dotnet\shared\Microsoft.NETCore.App\3.1.0. There appears to be one for every installed SDK version. And this one is an implementation of netstandard2.1.
Of course, for SDKs from .NET Core 2.x, the implementation is netstandard2.0.
There is also a standalone reference to .NET Standard 2.1 in C:\Program Files\dotnet\packs\NETStandard.Library.Ref\2.1.0\ref\netstandard2.1.
Both seem to get me past the netstandard error.
There was a problem hiding this comment.
Okay, after doing it this way (excluding -static and -classloader:), it works. It seems that the -lib:<ikvmtools>/refs option somehow causes it to search for netstandard.dll, but using -r:<ikvmtools>/refs/*.dll does not.
|
Implemented in develop. |
#65 describes an issue where ikvmc out of the box, on Core, isn't really able to trace down it's reference assemblies. This patch replaces the weird logic attempting to append directory names together with code that uses DependencyContext to search for reference assemblies instead. This should cause it to search in the right location.
This is an improvement over the current code, but it's still not quite right. Universe shouldn't be the place responsible for this, instead ikvmc should be adding the paths to Universe as it sees fit for it's goals, and only in the ~-nostdlib situation. But the concerns of IKVM.Runtime and ikvmc got a bit confused at some point in the past it looks like. This is at least better.