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.

Sign up for GitHub

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

Remove the magic "ReferenceAssembliesDirectory" #67

Closed
wants to merge 13 commits into from
Closed

Remove the magic "ReferenceAssembliesDirectory" #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
033ab80
Remove the magic "ReferenceAssembliesDirectory" and replace with usag…
wasabii May 30, 2022
5a0811f
Added ikvmc.Tests project, which runs on netcore3.1, but includes pub…
wasabii May 30, 2022
d2bd5fd
Added a dynamic data test that runs once per artifact build, and diff…
wasabii May 30, 2022
9eec400
Use CliWrap.
wasabii May 30, 2022
58745ba
Verify exit code option.
wasabii May 30, 2022
ec8eda7
Removing the -nostdlib option. Relying exclusively on specifying refe…
wasabii May 30, 2022
de89394
Check private and public package counts before using.
wasabii May 31, 2022
8f93e85
The tests do come with the compilation context, so they can run ikvmc…
wasabii May 31, 2022
5fc550b
Removed LegacyAssemblyResolve. It appears this was used for Stub gene…
wasabii May 31, 2022
1150fd0
Hmm
wasabii May 31, 2022
1268fc9
Merge branch 'develop' into bug/65
wasabii May 31, 2022
e9d2f24
Fix copyright issue.
wasabii May 31, 2022
e696552
Hmm
wasabii Jun 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,55 @@
</MSBuild>
</Target>

<ItemGroup>
<ProjectReference Include="@(PublishProjectReference)">
<Private>false</Private>
<IncludeAssets>none</IncludeAssets>
<ExcludeAssets>all</ExcludeAssets>
<ReferenceOutputAssembly>false</ReferenceOutputAssembly>
<CopyLocalSatelliteAssemblies>false</CopyLocalSatelliteAssemblies>
<SetTargetFramework>%(SetTargetFramework)</SetTargetFramework>
<PublishTargetPath>%(TargetPath)</PublishTargetPath>
<PublishProperties>%(Properties)</PublishProperties>
<Targets></Targets>
</ProjectReference>
</ItemGroup>

<Target Name="GetPublishProjectReferences" DependsOnTargets="ResolveProjectReferences">
<ItemGroup>
<_PublishProjectReference Include="@(ProjectReferenceWithConfiguration)" Condition="'%(ProjectReferenceWithConfiguration.PublishTargetPath)' != ''">
<ProjectName>$([System.IO.Path]::GetFileNameWithoutExtension('%(Identity)'))</ProjectName>
<PublishTargetPath>$([MSBuild]::EnsureTrailingSlash(%(ProjectReferenceWithConfiguration.PublishTargetPath)))</PublishTargetPath>
</_PublishProjectReference>
</ItemGroup>
</Target>

<Target Name="GetPublishProjectReferenceOutputItems" DependsOnTargets="GetPublishProjectReferences" Inputs="@(_PublishProjectReference)" Outputs="%(PublishProjectReference.Identity)\dummy">
<MSBuild Projects="@(_PublishProjectReference)" Targets="Publish;PublishItemsOutputGroup" BuildInParallel="$(BuildInParallel)" Properties="%(_PublishProjectReference.SetConfiguration);%(_PublishProjectReference.SetPlatform);%(_PublishProjectReference.SetTargetFramework);%(_PublishProjectReference.PublishProperties)" RemoveProperties="%(_PublishProjectReference.GlobalPropertiesToRemove);%(_PublishProjectReference.PublishPropertiesToRemove);" RebaseOutputs="true">
<Output TaskParameter="TargetOutputs" ItemName="_PublishProjectReferenceOutputItems" />
</MSBuild>
</Target>

<Target Name="_GetPublishProjectReferenceCopyToOutputDirectoryItems" DependsOnTargets="GetPublishProjectReferenceOutputItems">
<ItemGroup>
<ContentWithTargetPath Include="@(_PublishProjectReferenceOutputItems)" Condition=" '%(_PublishProjectReferenceOutputItems.TargetPath)' != '' ">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
<TargetPath>%(_PublishProjectReferenceOutputItems.PublishTargetPath)\%(_PublishProjectReferenceOutputItems.TargetPath)</TargetPath>
</ContentWithTargetPath>
</ItemGroup>
</Target>

<PropertyGroup>
<GetPublishProjectReferenceCopyToOutputDirectoryItemsDependsOn>
$(GetPublishProjectReferenceCopyToOutputDirectoryItemsDependsOn);
GetPublishProjectReferences;
GetPublishProjectReferenceOutputItems;
_GetPublishProjectReferenceCopyToOutputDirectoryItems;
</GetPublishProjectReferenceCopyToOutputDirectoryItemsDependsOn>
</PropertyGroup>

<Target Name="GetPublishProjectReferenceCopyToOutputDirectoryItems" DependsOnTargets="$(GetPublishProjectReferenceCopyToOutputDirectoryItemsDependsOn)" BeforeTargets="GetCopyToOutputDirectoryItems">

</Target>

</Project>
5 changes: 2 additions & 3 deletions IKVM.Java/IKVM.Java.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,8 @@
</PropertyGroup>

<MakeDir Directories="$(RefStubsOutputPath);$(RefStubsOutputPath)tmp" />
<Message Text="$(_IkvmToolExecPrefix)$(IkvmStub) -bootstrap &quot;%(_RefStubWorkItem.AssemblyFile)&quot; $(_RefStubArgs) -nostdlib -out:&quot;%(TmpFile)&quot;" />
<Exec Command="$(_IkvmToolExecPrefix)$(IkvmStub) -bootstrap &quot;%(_RefStubWorkItem.AssemblyFile)&quot; $(_RefStubArgs) -nostdlib -out:&quot;%(TmpFile)&quot;" />
<Message Text="$(_IkvmToolExecPrefix)$(IkvmStub) -bootstrap &quot;%(_RefStubWorkItem.AssemblyFile)&quot; $(_RefStubArgs) -out:&quot;%(TmpFile)&quot;" />
<Exec Command="$(_IkvmToolExecPrefix)$(IkvmStub) -bootstrap &quot;%(_RefStubWorkItem.AssemblyFile)&quot; $(_RefStubArgs) -out:&quot;%(TmpFile)&quot;" />

<CalculateMD5ForRefStubs Source="@(_RefStubWorkItem)">
<Output TaskParameter="Output" ItemName="_RefStubWorkItemWithHash" />
Expand Down Expand Up @@ -725,7 +725,6 @@
<_IkvmcArgs Include="-strictfinalfieldsemantics" />
<_IkvmcArgs Include="-removeassertions" />
<_IkvmcArgs Include="-target:library" />
<_IkvmcArgs Include="-nostdlib" />
<_IkvmcArgs Include="-sharedclassloader" />
<_IkvmcArgs Include="-nowarn:110" />
<_IkvmcArgs Include="-w4" />
Expand Down
2 changes: 1 addition & 1 deletion IKVM.Reflection/Fusion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ internal static bool CompareAssemblyIdentityPure(string assemblyIdentity1, bool
throw new ArgumentException();
}

if (name2.Name != null && name2.Name.Equals(Universe.CoreLibName, StringComparison.OrdinalIgnoreCase))
if (name2.Name != null && name2.Name.Equals(Universe.StdLibName, StringComparison.OrdinalIgnoreCase))
{
if (name1.Name != null && name1.Name.Equals(name2.Name, StringComparison.OrdinalIgnoreCase))
{
Expand Down
166 changes: 36 additions & 130 deletions IKVM.Reflection/Universe.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ Jeroen Frijters
using System;
using System.Collections.Generic;
using System.IO;
using System.Runtime.InteropServices;
using System.Linq;
using System.Security;
using System.Text;

using IKVM.Reflection.Emit;
using IKVM.Reflection.Reader;

#if NETCOREAPP3_1_OR_GREATER
using Microsoft.Extensions.DependencyModel;
#endif

namespace IKVM.Reflection
{

Expand Down Expand Up @@ -145,44 +149,13 @@ public enum UniverseOptions
public sealed class Universe : IDisposable
{

#if NETCOREAPP3_1
#if NETCOREAPP3_1_OR_GREATER

public static readonly string CoreLibName = "netstandard";

public static string ReferenceAssembliesDirectory
{
get
{
return BuildRefDirFrom(System.Runtime.InteropServices.RuntimeEnvironment.GetRuntimeDirectory());
}
}

private static string BuildRefDirFrom(string runtimeDir)
{
// transform a thing like
// C:\Program Files\dotnet\shared\Microsoft.NETCore.App\3.1.7
// to
// C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\3.1.0\ref\netcoreapp3.1

var parts = runtimeDir.Split(Path.DirectorySeparatorChar);
var n = string.IsNullOrEmpty(parts[parts.Length - 1]) ? parts.Length - 2 : parts.Length - 1;
var versionDir = parts[n--];
var frameworkDir = parts[n--];
var newParts = new string[n + 5];
Array.Copy(parts, newParts, n);
var suffixParts = new string[] { "packs", frameworkDir + ".Ref", "3.1.0", "ref", "netcoreapp3.1" };
Array.Copy(suffixParts, 0, newParts, n, suffixParts.Length);
var dir = Path.Combine(newParts);
if (!Directory.Exists(dir))
{
throw new FileNotFoundException("Reference assemblies directory: " + dir);
}
return dir;
}
public static readonly string StdLibName = "netstandard";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.Runtime at design time and System.Private.CoreLib at 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there are type forwards in it, and the code path doesn't differentiate between the static and dynamic compilers. It is not optimal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@wasabii wasabii Jun 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I had thought netstandard was around in every core sdk for compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can I ask how to use it?


#elif NETFRAMEWORK || MONO

public static readonly string CoreLibName = "mscorlib";
public static readonly string StdLibName = "mscorlib";

#endif

Expand Down Expand Up @@ -286,7 +259,7 @@ private static bool GetUseNativeFusion()

internal Assembly Mscorlib
{
get { return Load(CoreLibName); }
get { return Load(StdLibName); }
}

private Type ImportMscorlibType(string ns, string name)
Expand Down Expand Up @@ -567,7 +540,7 @@ internal Type System_Security_Permissions_SecurityAction

internal bool HasMscorlib
{
get { return GetLoadedAssembly(CoreLibName) != null; }
get { return GetLoadedAssembly(StdLibName) != null; }
}

public event ResolveEventHandler AssemblyResolve
Expand Down Expand Up @@ -782,108 +755,41 @@ private Assembly GetDynamicAssembly(string refname)
return null;
}

public Assembly Load(string refname)
public Assembly Load(string refName)
{
return Load(refname, null, true);
return Load(refName, null, true);
}

internal Assembly Load(string refname, Module requestingModule, bool throwOnError)
internal Assembly Load(string refName, Module requestingModule, bool throwOnError)
{
Assembly asm = GetLoadedAssembly(refname);
if (asm != null)
{
return asm;
}
if (resolvers.Count == 0)
{
asm = DefaultResolver(refname, throwOnError);
}
else
{
ResolveEventArgs args = new ResolveEventArgs(refname, requestingModule == null ? null : requestingModule.Assembly);
foreach (ResolveEventHandler evt in resolvers)
{
asm = evt(this, args);
if (asm != null)
{
break;
}
}
if (asm == null)
{
asm = GetDynamicAssembly(refname);
}
}
if (asm != null)
{
string defname = asm.FullName;
if (refname != defname)
{
assembliesByName.Add(refname, asm);
}
return asm;
}
if (throwOnError)
{
throw new FileNotFoundException(refname);
}
return null;
}
// if the assembly is already loaded, just return that
var assembly = GetLoadedAssembly(refName);
if (assembly != null)
return assembly;

private Assembly DefaultResolver(string refname, bool throwOnError)
{
Assembly asm = GetDynamicAssembly(refname);
if (asm != null)
{
return asm;
}
// dispatch a resolve event to the resolvers
var arg = new ResolveEventArgs(refName, requestingModule?.Assembly);
assembly = resolvers.Select(i => i(this, arg)).FirstOrDefault(i => i != null);

// still not found, maybe it's a dynamic assembly?
if (assembly == null)
assembly = GetDynamicAssembly(refName);

#if NETCOREAPP3_1
string filepath = Path.Combine(ReferenceAssembliesDirectory, GetSimpleAssemblyName(refname) + ".dll");
if (File.Exists(filepath))
// we did find the assembly
if (assembly != null)
{
using (RawModule module = OpenRawModule(filepath))
{
AssemblyComparisonResult result;
if (module.IsManifestModule && CompareAssemblyIdentity(refname, false, module.GetAssemblyName().FullName, false, out result))
{
return LoadAssembly(module);
}
}
// associate the assembly with our set by name
var defName = assembly.FullName;
if (refName != defName)
assembliesByName.Add(refName, assembly);

return assembly;
}

if (throwOnError)
throw new FileNotFoundException(refName);

return null;
#else
string fileName;
if (throwOnError)
{
try
{
fileName = System.Reflection.Assembly.ReflectionOnlyLoad(refname).Location;
}
catch (System.BadImageFormatException x)
{
throw new BadImageFormatException(x.Message, x);
}
}
else
{
try
{
fileName = System.Reflection.Assembly.ReflectionOnlyLoad(refname).Location;
}
catch (System.BadImageFormatException x)
{
throw new BadImageFormatException(x.Message, x);
}
catch (FileNotFoundException)
{
// we intentionally only swallow the FileNotFoundException, if the file exists but isn't a valid assembly,
// we should throw an exception
return null;
}
}
return LoadFile(fileName);
#endif
}

public Type GetType(string assemblyQualifiedTypeName)
Expand Down
29 changes: 13 additions & 16 deletions IKVM.Runtime/ClassFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ internal ClassFile(byte[] buf, int offset, int length, string inputClassName, Cl
GetConstantPoolClass(class_index),
null,
null
};
};
}
else
{
Expand Down Expand Up @@ -767,16 +767,16 @@ private static object ReadAnnotationElementValue(BigEndianBinaryReader rdr, Clas
ushort type_name_index = rdr.ReadUInt16();
ushort const_name_index = rdr.ReadUInt16();
return new object[] {
AnnotationDefaultAttribute.TAG_ENUM,
classFile.GetConstantPoolUtf8String(utf8_cp, type_name_index),
classFile.GetConstantPoolUtf8String(utf8_cp, const_name_index)
};
AnnotationDefaultAttribute.TAG_ENUM,
classFile.GetConstantPoolUtf8String(utf8_cp, type_name_index),
classFile.GetConstantPoolUtf8String(utf8_cp, const_name_index)
};
}
case (byte)'c':
return new object[] {
AnnotationDefaultAttribute.TAG_CLASS,
classFile.GetConstantPoolUtf8String(utf8_cp, rdr.ReadUInt16())
};
AnnotationDefaultAttribute.TAG_CLASS,
classFile.GetConstantPoolUtf8String(utf8_cp, rdr.ReadUInt16())
};
case (byte)'@':
return ReadAnnotation(rdr, classFile, utf8_cp);
case (byte)'[':
Expand Down Expand Up @@ -1241,15 +1241,12 @@ internal string SourcePath
#endif
}

internal object[] Annotations
{
get
{
return annotations;
}
}
/// <summary>
/// Gets the annotations set on the class.
/// </summary>
internal object[] Annotations => annotations;

internal string GenericSignature
internal string GenericSignature
{
get
{
Expand Down
12 changes: 6 additions & 6 deletions IKVM.Runtime/ClassLoaderWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,12 +1109,12 @@ internal static TypeWrapper GetWrapperFromType(Type type)
}

#if STUB_GENERATOR
if(type.__IsMissing || type.__ContainsMissingType)
{
wrapper = new UnloadableTypeWrapper("Missing/" + type.Assembly.FullName);
globalTypeToTypeWrapper.Add(type, wrapper);
return wrapper;
}
if (type.__IsMissing || type.__ContainsMissingType)
{
wrapper = new UnloadableTypeWrapper("Missing/" + type.Assembly.FullName);
globalTypeToTypeWrapper.Add(type, wrapper);
return wrapper;
}
#endif
string remapped;
if (remappedTypes.TryGetValue(type, out remapped))
Expand Down
Loading