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

[msbuild] Add reference to System.Drawing.Common.dll to XI projects. #6011

Merged
merged 7 commits into from
May 9, 2019
Merged

[msbuild] Add reference to System.Drawing.Common.dll to XI projects. #6011

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9279664
[msbuild] Add reference to `System.Drawing.Common.dll` to XI projects.
radical May 8, 2019
6f70dbf
[msbuild] Implement the same fix for XM projects as well.
rolfbjarne May 8, 2019
3d8eb8b
[msbuild] Update Xamarin.iOS.Tasks.TargetTests.GetReferencedAssemblie…
rolfbjarne May 8, 2019
4211284
[tests] Adjust Xamarin.MMP.Tests.AssemblyReferencesTests.ShouldNotAll…
rolfbjarne May 8, 2019
5bafdd4
Remove workarounds for mono/mono#13483.
rolfbjarne May 8, 2019
bae0abb
[msbuild] Create a way out if automatically referencing System.Drawin…
rolfbjarne May 9, 2019
d7838a3
[msbuild] Adjust variable name and boolean logic according to review.
rolfbjarne May 9, 2019
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
7 changes: 7 additions & 0 deletions msbuild/Xamarin.Mac.Tasks/Xamarin.Mac.Common.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,13 @@ Copyright (C) 2014 Xamarin. All rights reserved.
<RemoveDir Directories="$(IntermediateOutputPath)" />
</Target>

<Target Name="_AddExtraReferences" BeforeTargets="ResolveAssemblyReferences" Condition="'$(DisableAutomaticSystemDrawingCommonReference)' == ''">
Copy link
Contributor

Choose a reason for hiding this comment

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

I think DisableAutomaticSystemDrawingCommonReference should have a generic name to match the target name like DisableExtraReferences or DisableAutomaticExtraReferences.

Besides that, since it's a boolean property it'd be better give it a default value if it's empty (set it to false) or check if it's != 'true', because if someone sets it to false for any reason the target will be skipped when it shouldn't.

Copy link
Member

Choose a reason for hiding this comment

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

@emaf ok, I've changed it to use DisableExtraReferences and check for (not) true instead of empty string.

<ItemGroup>
<!-- https://github.com/mono/mono/issues/13483 -->
<Reference Include="System.Drawing.Common.dll" />
</ItemGroup>
</Target>

<PropertyGroup>
<_CollectBundleResourcesDependsOn>
_CompileImageAssets;
Expand Down
7 changes: 7 additions & 0 deletions msbuild/Xamarin.iOS.Tasks.Core/Xamarin.iOS.Common.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,13 @@ Copyright (C) 2013-2016 Xamarin. All rights reserved.
<Delete SessionId="$(BuildSessionId)" Condition="'$(IsMacEnabled)' == 'true'" Files="@(_IpaPackageFile)" />
</Target>

<Target Name="_AddExtraReferences" BeforeTargets="ResolveAssemblyReferences" Condition="'$(DisableAutomaticSystemDrawingCommonReference)' == ''">
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing than in the Mac common targets

<ItemGroup>
<!-- https://github.com/mono/mono/issues/13483 -->
<Reference Include="System.Drawing.Common.dll" />
</ItemGroup>
</Target>

<PropertyGroup>
<_CollectBundleResourcesDependsOn>
_CompileInterfaceDefinitions;
Expand Down
38 changes: 24 additions & 14 deletions msbuild/tests/Xamarin.iOS.Tasks.Tests/TargetTests/TargetTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,28 +139,38 @@ public void GetReferencedAssemblies_Executable ()
{
RunTargetOnInstance (MonoTouchProjectInstance, TargetName.ResolveReferences);
var references = MonoTouchProjectInstance.GetItems ("ReferencePath").ToArray ();
var expected_references = new string[] {
"MyLibrary.dll",
"System.dll",
"System.Xml.dll",
"System.Core.dll",
"mscorlib.dll",
"Xamarin.iOS.dll",
"System.Drawing.Common.dll",
};
Array.Sort (expected_references);

Assert.AreEqual (6, references.Length, "#1");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("MyLibrary")), "#2");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("System")), "#3a");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("System.Xml")), "#3b");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("System.Core")), "#3c");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("mscorlib")), "#3d");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("Xamarin.iOS")), "#3e");
var actual_references = references.Select ((v) => Path.GetFileName (v.EvaluatedInclude)).OrderBy ((v) => v);
CollectionAssert.AreEquivalent (expected_references, actual_references, "References");
}

[Test]
public void GetReferencedAssemblies_Library ()
{
RunTargetOnInstance (LibraryProjectInstance, TargetName.ResolveReferences);
var references = LibraryProjectInstance.GetItems ("ReferencePath").ToArray ();

Assert.AreEqual (5, references.Length, "#1");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("System")), "#2a");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("System.Xml")), "#2b");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("System.Core")), "#2c");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("mscorlib")), "#2d");
Assert.IsTrue (references.Any (t => t.EvaluatedInclude.Contains ("Xamarin.iOS")), "#2e");
var expected_references = new string[] {
"System.dll",
"System.Xml.dll",
"System.Core.dll",
"mscorlib.dll",
"Xamarin.iOS.dll",
"System.Drawing.Common.dll",
};
Array.Sort (expected_references);

var actual_references = references.Select ((v) => Path.GetFileName (v.EvaluatedInclude)).OrderBy ((v) => v);
CollectionAssert.AreEquivalent (expected_references, actual_references, "References");
}

[Test]
Expand Down
16 changes: 13 additions & 3 deletions tests/common/mac/ProjectTestHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,18 @@ public class UnifiedTestConfig
// Generated by TestUnifiedExecutable/TestSystemMonoExecutable and added to TestCode
public Guid guid { get; set; }

public string BundleName {
get { return AssemblyName != "" ? AssemblyName : ProjectName.Split ('.') [0]; }
}

public string BundlePath {
get { return Path.Combine (TmpDir, "bin", Release ? "Release" : "Debug", BundleName + ".app"); }
}

public string ExecutablePath {
get { return Path.Combine (BundlePath, "Contents", "MacOS", BundleName); }
}

public UnifiedTestConfig (string tmpDir)
{
TmpDir = tmpDir;
Expand Down Expand Up @@ -427,9 +439,7 @@ public static string GenerateAndBuildUnifiedExecutable (UnifiedTestConfig config

public static string RunGeneratedUnifiedExecutable (UnifiedTestConfig config)
{
string bundleName = config.AssemblyName != "" ? config.AssemblyName : config.ProjectName.Split ('.')[0];
string exePath = Path.Combine (config.TmpDir, "bin/" + (config.Release ? "Release/" : "Debug/") + bundleName + ".app/Contents/MacOS/" + bundleName);
return RunEXEAndVerifyGUID (config.TmpDir, config.guid, exePath);
return RunEXEAndVerifyGUID (config.TmpDir, config.guid, config.ExecutablePath);
}

public static OutputText TestUnifiedExecutable (UnifiedTestConfig config, bool shouldFail = false, string[] environment = null)
Expand Down
1 change: 0 additions & 1 deletion tests/introspection/iOS/introspection-ios.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@
<Reference Include="System" />
<Reference Include="System.Core" />
<Reference Include="System.Data" />
<Reference Include="System.Drawing.Common" /> <!-- FIXME: https://github.com/mono/mono/issues/13483 -->
<Reference Include="Xamarin.iOS" />
<Reference Include="MonoTouch.NUnitLite" />
<Reference Include="System.Core" />
Expand Down
7 changes: 6 additions & 1 deletion tests/mmptest/src/AssemblyReferencesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.IO;
using System.Linq;
using System.Text;
using NUnit.Framework;

Expand All @@ -17,7 +18,11 @@ public void ShouldNotAllowReference_ToSystemDrawing ()
TestCode = "System.Drawing.RectangleF f = new System.Drawing.RectangleF ();",
XM45 = true
};
TI.TestUnifiedExecutable (test, shouldFail: true);
TI.TestUnifiedExecutable (test);
var allAssembliesInBundle = Directory.GetFiles (test.BundlePath, "*.dll", SearchOption.AllDirectories).Select (Path.GetFileName);
Assert.That (allAssembliesInBundle, Does.Contain ("mscorlib.dll"), "mscorlib.dll");
Assert.That (allAssembliesInBundle, Does.Contain ("System.Drawing.Common.dll"), "System.Drawing.Common.dll");
Assert.That (allAssembliesInBundle, Does.Not.Contain ("System.Drawing.dll"), "System.Drawing.dll");
});
}

Expand Down
1 change: 0 additions & 1 deletion tests/monotouch-test/monotouch-test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@
<Reference Include="MonoTouch.NUnitLite" />
<Reference Include="System.Net.Http" />
<Reference Include="OpenTK-1.0" />
<Reference Include="System.Drawing.Common" /> <!-- FIXME: https://github.com/mono/mono/issues/13483 -->
</ItemGroup>
<ItemGroup>
<None Include="Info.plist">
Expand Down
1 change: 0 additions & 1 deletion tests/xammac_tests/xammac_tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
</Reference>
<Reference Include="System.Net.Http" />
<Reference Include="OpenTK" />
<Reference Include="System.Drawing.Common" /> <!-- FIXME: https://github.com/mono/mono/issues/13483 -->
<Reference Include="System.Xml" />
</ItemGroup>
<ItemGroup>
Expand Down