use cecil repo instead of nuget package #101

radekdoulik · 2016-11-16T16:58:59Z

the reason to use the Cecil repo is to be quicker when fixing Cecil
bugs (usually hit from the XA linker)
added "proxy" projects for Xamarin.Android.Cecil and
Xamarin.Android.Cecil.Mdb libraries
the logic was moved from monodroid repo, because Java.Interop is
1st user of Cecil during the Xamarin Android build process
updated all the projects to use the Xamarin.Android.Cecil library
instead of Mono.Cecil nuget package

jonpryor · 2016-11-16T17:12:34Z

src/Xamarin.Android.Cecil/Mono.Cecil.overrides

+  <PropertyGroup>
+	<AssemblyName>$(AssemblyName.Replace('Mono', 'Xamarin.Android'))</AssemblyName>
+  </PropertyGroup>
+	<ItemGroup>


Indentation should be consistent in this file. Either everything should use spaces, or everything should use tabs (use spaces!). Regardless, <PropertyGroup/> and <ItemGroup/> should vertically align, and they don't here.

Ah, good catch. This files was moved from monodroid repo. It is good opportunity to fix the indentation, done.

jonpryor · 2016-11-16T17:15:51Z

Java.Interop.sln

+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Xamarin.Android.Cecil", "src\Xamarin.Android.Cecil\Xamarin.Android.Cecil.csproj", "{15945D4B-FF56-4BCC-B598-2718D199DD08}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Xamarin.Android.Cecil.Mdb", "src\Xamarin.Android.Cecil\Xamarin.Android.Cecil.Mdb.csproj", "{C0487169-8F81-497F-919E-EB42B1D0243F}"


Should this go into a src\Xamarin.Android.Cecil.Mdb directory? I'm not sure.

I wanted to keep it together in one place, so hopefully it is OK to keep here.

jonpryor · 2016-11-16T17:25:27Z

src/Xamarin.Android.Cecil/Xamarin.Android.Cecil.targets

+  <Target Name="PrepareCecil"
+      Inputs="$(CecilDirectory)\Mono.Cecil.sln"
+      Outputs="$(CecilPreparedFlag)">
+    <Exec Command="cd $(CecilDirectory); ln -sf $(MSBuildThisFileDirectory)\AssemblyInfo.cs Mono.Cecil.AssemblyInfo.cs" />


A medium-term (short-term?) goal for the Java.Interop and xamarin-android repos is to "support" building the projects on Windows.

We shouldn't deliberately introduce anything that would prevent that from working. It might not work for other reasons -- e.g. mono builds on Windows are something I'm trying to avoid in xamarin-android, hence the bundle generation & caching -- but we shouldn't deliberately introduce new constructs which we know won't work on Windows.

ln -s is such a construct. Perhaps it should use the <Copy/> task?

Unfortunately, patch is also such a construct. I'm not sure how best to handle that; I guess we should keep it for now.

OK, I used Copy instead and added the files to Inputs so that the target is rebuilt when these are changed.

jonpryor · 2016-11-16T17:28:12Z

...n.Android.Tools.AnnotationSupport.Cecil/Xamarin.Android.Tools.AnnotationSupport.Cecil.csproj

      <Name>Xamarin.Android.Tools.AnnotationSupport</Name>
    </ProjectReference>
  </ItemGroup>
+  <ItemGroup>


Odd that this <ItemGroup/> was re-ordered...

I think that remained there after my small fight with msbuild :-) Fixed.

radekdoulik · 2016-11-17T17:10:04Z

OK, the patch is updated and hopefully ready to merge.

jonpryor · 2016-11-17T20:09:44Z

src/Xamarin.Android.Cecil/Xamarin.Android.Cecil.targets

+      Outputs="$(CecilPreparedFlag)">
+    <Copy SourceFiles="AssemblyInfo.cs" DestinationFiles="$(CecilDirectory)\Mono.Cecil.AssemblyInfo.cs" />
+    <Copy SourceFiles="Mono.Cecil.overrides" DestinationFolder="$(CecilDirectory)" />
+    <Exec Command="cd $(CecilDirectory); git reset --hard" />


This shouldn't be necessary.

Instead, we should add a top-level make prepare target, a'la xamarin-android, and should also contain/handle the nuget restore logic.

- the reason to use the Cecil repo is to be quicker when fixing Cecil bugs (usually hit from the XA linker) - added "proxy" projects for Xamarin.Android.Cecil and Xamarin.Android.Cecil.Mdb libraries - the logic was moved from monodroid repo, because Java.Interop is 1st user of Cecil during the Xamarin Android build process - updated all the projects to use the Xamarin.Android.Cecil library instead of Mono.Cecil nuget package

kzu · 2016-11-18T12:42:18Z

Changing the assembly name of Cecil does not require proxy projects or assembly patching. See dotnet/cecil#6.

On XVS, we just add the overrides targets to our external root folder, like so: https://github.com/xamarin/XamarinVS/blob/master/External/Mono.Cecil.overrides.

And we also add InternalsVisisbleTo accordingly: https://github.com/xamarin/XamarinVS/blob/master/External/Cecil.AssemblyInfo.cs

Use cecil repo instead of nuget package. The reason to use the Cecil repo is to be quicker when fixing Cecil bugs (usually hit from the XA linker). Added "proxy" projects for `Xamarin.Android.Cecil` and `Xamarin.Android.Cecil.Mdb` libraries. We use these names because they match the currently used "Vendorized" Cecil which is distributed with commercial Xamarin.Android. Updated all the projects to use the `Xamarin.Android.Cecil` library instead of `Mono.Cecil` nuget package.

Context: https://medium.com/@alex.birsan/dependency-confusion-4a5d60fec610 Context: https://azure.microsoft.com/en-us/resources/3-ways-to-mitigate-risk-using-private-package-feeds/ Context: https://devdiv.visualstudio.com/DevDiv/_wiki/wikis/DevDiv.wiki/12676/ncident-help-for-Substitution-attack-risk-from-multiple-package-feeds Changes: dotnet/android-tools@26d65d9...63510cf * dotnet/android-tools@63510cf: [ci] Update packageSources in NuGet.config (dotnet#105) * dotnet/android-tools@83ed0a4: Bump ta xamarin/LibZipSharp/1.0.22@9f563dd1 (dotnet#104) * dotnet/android-tools@8ea78a4: Add Microsoft.Android.Build.BaseTasks project (dotnet#101) * dotnet/android-tools@b2d9fdf: [NDK] Locate and select only compatible NDK versions (dotnet#103) * dotnet/android-tools@5ff1702: [tests] Use dotnet test to run AndroidSdk-Tests (dotnet#102) * dotnet/android-tools@ad80a42: [ci] Use the new "main" default branch (dotnet#100) There is a Package Substitution Attack inherent in NuGet, whereby if multiple package sources provide packages with the same name, it is *indeterminate* which package source will provide the package. For example, consider the [`XliffTasks` package][0], currently provided from the [`dotnet-eng`][1] feed, and *not* present in the NuGet.org feed. If a "hostile attacker" submits an `XliffTasks` package to NuGet.org, then we don't know, and cannot control, whether the build will use the "hostile" `XliffTasks` package from NuGet.org or the "desired" package from `dotnet-eng`. There are two ways to prevent this attack: 1. Use `//packageSources/clear` and have *only one* `//packageSources/add` entry in `NuGet.config` 2. Use `//packageSources/clear` and *fully trust* every `//packageSources/add` entry in `NuGet.config`. `NuGet.org` *cannot* be a trusted source, nor can any feed location which allows "anyone" to add new packages, nor can a feed which itself contains [upstream sources][2]. As the `XliffTasks` package is *not* in `NuGet.org`, option (1) isn't an option. Go with option (2), using the existing `dotnet-eng` source and the new *trusted* [`dotnet-public`][3] package source. [0]: https://github.com/dotnet/xliff-tasks [1]: https://dev.azure.com/dnceng/public/_packaging?_a=feed&feed=dotnet-eng [2]: https://docs.microsoft.com/en-us/azure/devops/artifacts/concepts/upstream-sources?view=azure-devops [3]: https://dev.azure.com/dnceng/public/_packaging?_a=feed&feed=dotnet-public

Context: https://medium.com/@alex.birsan/dependency-confusion-4a5d60fec610 Context: https://azure.microsoft.com/en-us/resources/3-ways-to-mitigate-risk-using-private-package-feeds/ Context: https://devdiv.visualstudio.com/DevDiv/_wiki/wikis/DevDiv.wiki/12676/ncident-help-for-Substitution-attack-risk-from-multiple-package-feeds Changes: dotnet/android-tools@26d65d9...479931c * dotnet/android-tools@479931c [build] Move global.json file to root directory (dotnet#106) * dotnet/android-tools@63510cf: [ci] Update packageSources in NuGet.config (dotnet#105) * dotnet/android-tools@83ed0a4: Bump ta xamarin/LibZipSharp/1.0.22@9f563dd1 (dotnet#104) * dotnet/android-tools@8ea78a4: Add Microsoft.Android.Build.BaseTasks project (dotnet#101) * dotnet/android-tools@b2d9fdf: [NDK] Locate and select only compatible NDK versions (dotnet#103) * dotnet/android-tools@5ff1702: [tests] Use dotnet test to run AndroidSdk-Tests (dotnet#102) * dotnet/android-tools@ad80a42: [ci] Use the new "main" default branch (dotnet#100) There is a Package Substitution Attack inherent in NuGet, whereby if multiple package sources provide packages with the same name, it is *indeterminate* which package source will provide the package. For example, consider the [`XliffTasks` package][0], currently provided from the [`dotnet-eng`][1] feed, and *not* present in the NuGet.org feed. If a "hostile attacker" submits an `XliffTasks` package to NuGet.org, then we don't know, and cannot control, whether the build will use the "hostile" `XliffTasks` package from NuGet.org or the "desired" package from `dotnet-eng`. There are two ways to prevent this attack: 1. Use `//packageSources/clear` and have *only one* `//packageSources/add` entry in `NuGet.config` 2. Use `//packageSources/clear` and *fully trust* every `//packageSources/add` entry in `NuGet.config`. `NuGet.org` *cannot* be a trusted source, nor can any feed location which allows "anyone" to add new packages, nor can a feed which itself contains [upstream sources][2]. As the `XliffTasks` package is *not* in `NuGet.org`, option (1) isn't an option. Go with option (2), using the existing `dotnet-eng` source and the new *trusted* [`dotnet-public`][3] package source. [0]: https://github.com/dotnet/xliff-tasks [1]: https://dev.azure.com/dnceng/public/_packaging?_a=feed&feed=dotnet-eng [2]: https://docs.microsoft.com/en-us/azure/devops/artifacts/concepts/upstream-sources?view=azure-devops [3]: https://dev.azure.com/dnceng/public/_packaging?_a=feed&feed=dotnet-public

…796) Context: https://medium.com/@alex.birsan/dependency-confusion-4a5d60fec610 Context: https://azure.microsoft.com/en-us/resources/3-ways-to-mitigate-risk-using-private-package-feeds/ Context: https://devdiv.visualstudio.com/DevDiv/_wiki/wikis/DevDiv.wiki/12676/ncident-help-for-Substitution-attack-risk-from-multiple-package-feeds Changes: dotnet/android-tools@26d65d9...479931c * dotnet/android-tools@479931c [build] Move global.json file to root directory (#106) * dotnet/android-tools@63510cf: [ci] Update packageSources in NuGet.config (#105) * dotnet/android-tools@83ed0a4: Bump ta xamarin/LibZipSharp/1.0.22@9f563dd1 (#104) * dotnet/android-tools@8ea78a4: Add Microsoft.Android.Build.BaseTasks project (#101) * dotnet/android-tools@b2d9fdf: [NDK] Locate and select only compatible NDK versions (#103) * dotnet/android-tools@5ff1702: [tests] Use dotnet test to run AndroidSdk-Tests (#102) * dotnet/android-tools@ad80a42: [ci] Use the new "main" default branch (#100) There is a Package Substitution Attack inherent in NuGet, whereby if multiple package sources provide packages with the same name, it is *indeterminate* which package source will provide the package. For example, consider the [`XliffTasks` package][0], currently provided from the [`dotnet-eng`][1] feed, and *not* present in the NuGet.org feed. If a "hostile attacker" submits an `XliffTasks` package to NuGet.org, then we don't know, and cannot control, whether the build will use the "hostile" `XliffTasks` package from NuGet.org or the "desired" package from `dotnet-eng`. There are two ways to prevent this attack: 1. Use `//packageSources/clear` and have *only one* `//packageSources/add` entry in `NuGet.config` 2. Use `//packageSources/clear` and *fully trust* every `//packageSources/add` entry in `NuGet.config`. `NuGet.org` *cannot* be a trusted source, nor can any feed location which allows "anyone" to add new packages, nor can a feed which itself contains [upstream sources][2]. As the `XliffTasks` package is *not* in `NuGet.org`, option (1) isn't an option. Go with option (2), using the existing `dotnet-eng` source and the new *trusted* [`dotnet-public`][3] package source. [0]: https://github.com/dotnet/xliff-tasks [1]: https://dev.azure.com/dnceng/public/_packaging?_a=feed&feed=dotnet-eng [2]: https://docs.microsoft.com/en-us/azure/devops/artifacts/concepts/upstream-sources?view=azure-devops [3]: https://dev.azure.com/dnceng/public/_packaging?_a=feed&feed=dotnet-public

dnfclas added the cla-already-signed label Nov 16, 2016

jonpryor reviewed Nov 16, 2016

View reviewed changes

radekdoulik force-pushed the use-cecil-repo branch from 84a6747 to 8e4afc2 Compare November 17, 2016 17:09

jonpryor reviewed Nov 17, 2016

View reviewed changes

radekdoulik force-pushed the use-cecil-repo branch from 8e4afc2 to 886e198 Compare November 17, 2016 21:11

radekdoulik force-pushed the use-cecil-repo branch from 886e198 to dac3608 Compare November 17, 2016 21:47

jonpryor merged commit f44a11f into dotnet:master Nov 17, 2016

jonpryor mentioned this pull request Feb 9, 2021

Bump to xamarin/xamarin-android-tools/main@63510cfc; packageSources #796

Merged

github-actions bot locked and limited conversation to collaborators Apr 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

use cecil repo instead of nuget package #101

use cecil repo instead of nuget package #101

Uh oh!

radekdoulik commented Nov 16, 2016

Uh oh!

jonpryor Nov 16, 2016

Uh oh!

radekdoulik Nov 17, 2016

Uh oh!

jonpryor Nov 16, 2016

Uh oh!

radekdoulik Nov 17, 2016

Uh oh!

jonpryor Nov 16, 2016

Uh oh!

radekdoulik Nov 17, 2016

Uh oh!

jonpryor Nov 16, 2016

Uh oh!

radekdoulik Nov 17, 2016

Uh oh!

radekdoulik commented Nov 17, 2016

Uh oh!

jonpryor Nov 17, 2016

Uh oh!

kzu commented Nov 18, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

use cecil repo instead of nuget package #101

use cecil repo instead of nuget package #101

Uh oh!

Conversation

radekdoulik commented Nov 16, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radekdoulik commented Nov 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kzu commented Nov 18, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants