Changes to the msbuild extensions targets that will enable copying the shims for net471 case #1712

joperezr · 2017-11-07T06:02:40Z

cc: @AlexGhiondea @weshaggard @dsplaisted @livarcocc

These are the required changes for copying the required shims in order to run a net471 app that contains assets that were built using the support package. An example of a case that is currently broken is the following:

A is a netstandard based library (could be ns1.5+)
B is a net461 library that has a dependency to A. It also uses the type HttpClient. (HttpClient is in one of the affected assemblies, there are 12 affected assemblies in total)
C is a net471 app that consumes B. When trying to run the app on net471, it will crash since it won't be able to find System.Net.Http.dll version 4.2.0.0.

The reason behind it is that since the net461 library depends on a NS1.5+ library, it will be built against the shims contained in the support package. These shims, have in some cases a higher assembly version than the one we have inbox on net471 (in 12 cases as I mentioned above). That means that your net461 library, will depend on a higher assembly version of a library than the ones present inbox on net471, which will cause a missing assembly error.

…e shims for net471 case

AlexGhiondea · 2017-11-07T22:17:40Z

build/DependencyVersions.props

@@ -13,7 +13,7 @@
    <NuGetVersion>4.5.0-preview2-4529</NuGetVersion>
    <NewtonsoftJsonVersion>9.0.1</NewtonsoftJsonVersion>
    <SystemReflectionMetadataVersion>1.4.2</SystemReflectionMetadataVersion>
-    <NETStandardLibraryNETFrameworkVersion>2.0.1-servicing-25708-01</NETStandardLibraryNETFrameworkVersion>
+    <NETStandardLibraryNETFrameworkVersion>2.0.1-servicing-25708-01</NETStandardLibraryNETFrameworkVersion> <!-- Once we have a new version of this package available, this version will have to be updated -->


This has to change in the cli repo as well. This only controls what the sdk uses to test itself

Noted, I'll make sure it changes in the CLI repo as well.

AlexGhiondea · 2017-11-07T22:24:03Z

...Microsoft/Microsoft.NET.Build.Extensions/Microsoft.NET.Build.Extensions.NETFramework.targets


    <!-- if any reference depends on netstandard and it is not inbox, add references and implementation assemblies for netstandard2.0  -->
-    <ItemGroup Condition="'$(DependsOnNETStandard)' == 'true' AND '$(NETStandardInbox)' != 'true'">
+    <ItemGroup Condition="'$(DependsOnNETStandard)' == 'true' AND '$(NETStandardInbox)' != 'true' AND '$(_TargetFrameworkVersionWithoutV)' != '4.7.1'">


< 4.7.1 ?

Or move the 4.7.1 special work in a single place.

joperezr · 2017-11-08T18:21:11Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToTargetNet471.cs

+                $"{testProject.Name}.pdb",
+                $"{netStandardProject.Name}.dll",
+                $"{netStandardProject.Name}.pdb",
+                "System.Diagnostics.DiagnosticSource.dll" // This library will get pulled in as part of the closure of the ns16 project and will be copyied because it's not inbox.


@AlexGhiondea I believe this might actually be a bug on the framework, so we might want to include this file inbox on 472.

Don't blindly include that inbox. Please consult with @vancem first as I know they were trying to keep it out of box.

I'd use the same comment here as in the previous test: // This is an implementation dependency of the System.Net.Http package, which won't get conflict resolved out

I'm also pretty sure we don't want to include this inbox. It's an implementation dependency of the OOB version of System.Net.Http. Conflict resolution will pick the support package of the Http library, but it doesn't know that this dependency is no longer needed.

If you want to consider making it so that this library isn't copied, please file a bug for that in the sdk repo.

Sounds good I'll update it as requested.

dsplaisted · 2017-11-08T20:47:19Z

...Microsoft/Microsoft.NET.Build.Extensions/Microsoft.NET.Build.Extensions.NETFramework.targets

+      .NET 4.7.1 has support for .NET Standard 2.0 built-in, so most of the facades aren't necessary.  However, the assembly versions of a few set of assemblies
+      do not yet match the ones shipped by the support package.  This means that the versions from the contract NuGet packages would be preferred to the in-box 
+      version (which is newer).  So if there is a dependency on netstandard.dll, we use the 4.2.0.0 version of the DLLs from the .NET Standard 2.0 "facades".
+      (Though these DLLs are not actually facades, they contain the implementation.)


Do all of them contain implementation? Are the version numbers for all of them 4.2.0.0?

No, some are partial facades, and some are full facades so I'll update the comment

dsplaisted · 2017-11-08T20:49:42Z

...Microsoft/Microsoft.NET.Build.Extensions/Microsoft.NET.Build.Extensions.NETFramework.targets

+
+    <!--
+      .NET 4.7.1 has support for .NET Standard 2.0 built-in, so most of the facades aren't necessary.  However, the assembly versions of a few set of assemblies
+      do not yet match the ones shipped by the support package.  This means that the versions from the contract NuGet packages would be preferred to the in-box 


I'd suggest updating this to say: "do not yet match the ones shipped in the support package for .NET Standard 2.0 on .NET 4.7 and below. This means that .NET 4.7 or lower libraries might have references to higher versions of these assemblies than are available in-box in .NET 4.7, leading to assembly loading errors."

will change.

dsplaisted · 2017-11-08T20:53:01Z

...Microsoft/Microsoft.NET.Build.Extensions/Microsoft.NET.Build.Extensions.NETFramework.targets


    <!-- if any reference depends on netstandard and it is not inbox, add references and implementation assemblies for netstandard2.0  -->
-    <ItemGroup Condition="'$(DependsOnNETStandard)' == 'true' AND '$(NETStandardInbox)' != 'true'">
+    <ItemGroup Condition="'$(DependsOnNETStandard)' == 'true' AND '$(NETStandardInbox)' != 'true' AND '$(_TargetFrameworkVersionWithoutV)' &lt; '4.7.1'">


Is the additional check that the target framework version is less than 4.7.1 necessary? NETStandardInbox == false should imply that the target framework version is less than 4.7.1.

Or is this there in case people explicitly set NETStandardInbox to false in their 4.7.1 projects as a workaround for this issue?

Or is this there in case people explicitly set NETStandardInbox to false in their 4.7.1 projects as a workaround for this issue?

Yes that is the case. Initially I had it just with != but got a comment saying that in case people override that value, we should make sure this logic doesn't kick in for 4.7.2+

dsplaisted · 2017-11-08T20:54:19Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToTargetNet471.cs

        public void It_builds_a_net471_app()
        {
+            //  https://github.com/dotnet/sdk/issues/1625
+            if (!Net471ReferenceAssembliesAreInstalled())


Did you run all these tests locally (with the reference assemblies installed)? We won't get any coverage for these tests on the CI machines.

Yes ran all, and they all passed. And I do have 4.7.1.

dsplaisted · 2017-11-08T21:03:27Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToTargetNet471.cs

@@ -127,14 +159,120 @@ public void It_does_not_include_facades_from_nuget_packages()
            outputDirectory.Should().OnlyHaveFiles(new[] {
                $"{testProject.Name}.exe",
                $"{testProject.Name}.pdb",
-
-                //  Remove these two once https://github.com/dotnet/sdk/issues/1647 is fixed
+
                "System.Net.Http.dll",
                "System.IO.Compression.dll",


I'd suggest adding a comment explaining why these two libraries are included. My understanding is that these were the only contracts from .NET Standard 1.x that have a higher version than what shipped in .NET 4.7.1.

yes, that is why. I'll add a comment.

dsplaisted · 2017-11-08T21:07:25Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToTargetNet471.cs

+                $"{testProject.Name}.pdb",
+                $"{netStandardProject.Name}.dll",
+                $"{netStandardProject.Name}.pdb",
+                "System.Diagnostics.DiagnosticSource.dll" // This library will get pulled in as part of the closure of the ns16 project and will be copyied because it's not inbox.


I'd use the same comment here as in the previous test: // This is an implementation dependency of the System.Net.Http package, which won't get conflict resolved out

I'm also pretty sure we don't want to include this inbox. It's an implementation dependency of the OOB version of System.Net.Http. Conflict resolution will pick the support package of the Http library, but it doesn't know that this dependency is no longer needed.

If you want to consider making it so that this library isn't copied, please file a bug for that in the sdk repo.

dsplaisted · 2017-11-08T21:08:48Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToTargetNet471.cs

+        }
+
+        [Fact]
+        public void It_does_not_include_shims_when_app_references_471_library()


I'd suggest also referencing a .NET 4.6.1 library in this test (either in addition to the 4.7.1 library, or making the test a Theory and running it for different referenced framework versions.

dsplaisted · 2017-11-08T21:10:26Z

test/Microsoft.NET.Build.Tests/GivenThatWeWantToTargetNet471.cs

+
+        static bool Net471ReferenceAssembliesAreInstalled()
+        {
+            var net461referenceAssemblies = ToolLocationHelper.GetPathToDotNetFrameworkReferenceAssemblies(TargetDotNetFrameworkVersion.Version461);


I'd suggest adding a comment explaining this, ie: The version of the MSBuild libraries we are referencing doesn't have an enum value for .NET 4.7.1. So we use the MSBuild API to find the path to the 4.6.1 reference assemblies, and locate the 4.7.1 reference assemblies relative to that.

dsplaisted · 2017-11-08T21:31:14Z

...Microsoft/Microsoft.NET.Build.Extensions/Microsoft.NET.Build.Extensions.NETFramework.targets

-      version (which is newer).  So if there is a dependency on netstandard.dll, we use the 4.2.0.0 version of the DLLs from the .NET Standard 2.0 "facades".
-      (Though these DLLs are not actually facades, they contain the implementation.)
+      do not yet match the ones shipped in the support package for .NET Standard 2.0 on .NET 4.7 and below. This means that .NET 4.7 or lower libraries might have
+      references to higher versions of these assemblies than are available in-box in .NET 4.7, leading to assembly loading errors.


Add this at the end: So if there is a dependency on netstandard.dll, we include DLLs for .NET 4.7.1 from the support package to avoid these errors.

mscrivo · 2018-02-07T19:26:06Z

Is this fully resolved? We ran into an issue where w3wp started crashing with ExecutionEngineException's after every build in VS 2017 15.5.6 after upgrading to .NET 4.7.1. When it didn't completely crash, it would also manifest as this exception when loading our web app:

[FileLoadException: Loading this assembly would produce a different grant set from other instances. (Exception from HRESULT: 0x80131401)]
   System.Signature.GetSignature(Void* pCorSig, Int32 cCorSig, RuntimeFieldHandleInternal fieldHandle, IRuntimeMethodInfo methodHandle, RuntimeType declaringType) +0
   System.Reflection.RuntimeConstructorInfo.GetParametersNoCopy() +86
   System.Reflection.RuntimeConstructorInfo.GetParameters() +14
   SimpleInjector.Advanced.AdvancedExtensions.Verify(IDependencyInjectionBehavior behavior, ConstructorInfo constructor) +28
   SimpleInjector.ContainerOptions.IsConstructableType(Type implementationType, String& errorMessage) +51
   SimpleInjector.Container.ThrowArgumentExceptionWhenTypeIsNotConstructable(Type implementationType, String parameterName) +35
   SimpleInjector.Container.Register(Type serviceType, Type implementationType, Lifestyle lifestyle, String serviceTypeParamName, String implementationTypeParamName) +114
   SimpleInjector.Container.Register(Type serviceType, Type implementationType, Lifestyle lifestyle) +45
   Nudge.Core.Client.GenericClient.Registry.Local(Container container) in C:\Code\nudge-app\src\Nudge.Core\Client\GenericClient\Registry.cs:10
   Nudge.Core.Injection.Registry`1.All(Container container) in C:\Code\nudge-app\src\Nudge.Core\Injection\Registry.cs:41
   Nudge.WebApp.Web.MvcApplication.Register() in C:\Code\nudge-app\src\Nudge.WebApp.Web\Global.asax.cs:45
   Nudge.WebApp.Web.MvcApplication.Application_Start() in C:\Code\nudge-app\src\Nudge.WebApp.Web\Global.asax.cs:23

And when inspecting the type it failed on, it was always one which used HttpClient

We have the above scenario almost exactly. .NET 4.7.1 top level projects (exe's, Web App's) calling into a mix of .NET 4.7.1 and .NET Standard 2.0 assemblies that make use of HttpClient from System.Net.Http.

You can see a chronicle of my journey in trying to resolve it here: simpleinjector/SimpleInjector#508. I don't at all like the workaround that I ended up with, which was using <ImplicitlyExpandNETStandardFacades>False</ImplicitlyExpandNETStandardFacades> in all top level projects and adding binding redirects for System.Net.Http to version 4.0.0.0. The end result is that we get build warnings now, but at least the app works and doesn't crash on startup, but I would like to remove these workarounds and get rid of the warnings if possible.

The version of System.Net.Http that gets copied into our bin folders when this happens, has the following properties:

File Version; 4.6.25908.2
Size: 193KB
Date Modified: 1/10/2018 1:38PM

When the workaround described above is applied, we end up with Version 4.6.24605.1 of System.Net.Http in our bin folders and everything works fine.

joperezr · 2018-02-07T20:31:03Z

So on VS 15.5.6 the right System.Net.Http will be copied to the output directory, the problem is that the tooling won't automatically add a binding redirect for you to it until VS 15.6 comes out. I'm sorry that you have to workaround this for now, but once we release 15.6 you should be able to remove that p roperty and remove the binding redirects since they will be calculated for you only for the ones that you actually need.

mscrivo · 2018-02-07T20:48:17Z

@joperezr Thank you for that confirmation. Glad it will be fixed soon and that I didn't do something wrong.

joperezr · 2018-02-07T20:50:37Z

Sorry for the trouble, and we will keep you posted once the fix comes out.

eriawan · 2018-02-12T07:23:47Z

@joperezr

I have VS 2017 15.5.5 and 15.5.6, and this issue is still there.
~~Is this fixed really made in to 15.5.6 timeframe or do I have to wait for 15.6 to be released?~~

Ah, apologize. I have missed the previous comment above.
I'll test this issue on 2017.15.6 Preview 4.0.

mscrivo · 2018-03-06T03:23:06Z

@joperezr I just installed the release version of 15.6 and the problem persists in .NET framework projects calling into .NET Standard projects with HttpClients crossing boundaries.

I've removed the <ImplicitlyExpandNETStandardFacades>False</ImplicitlyExpandNETStandardFacades> workaround, removed and re-added all binding redirects from scratch in our top level web projects by double clicking on the binding redirect errors in VS after the initial build. On subsequent builds, I got no warnings/errors (even did a git clean to make sure) and the above problems with FileLoadException and ExecutionEngineException's came back.

joperezr · 2018-03-06T17:13:33Z

@mscrivo Yes, I'm so sorry for the trouble. Because of some internal problems, the fix went in but it was too late to make it for VS 15.6. I have checked the preview build of 15.7 preview 2 and that one does contain the fix. We are actually in talks right now to try to get this specific issue serviced to 15.6.1 but is not yet clear if we will able to do it or not. Again, I'm so sorry for the trouble you are hitting, and we will work hard to get the fix to you ASAP.

mscrivo · 2018-03-06T17:58:55Z

Thanks @joperezr

mscrivo · 2018-03-28T20:33:33Z

@joperezr I just tried 15.7 Preview 2 just to make sure it's actually fixed, and it doesn't seem to be. I get the same behavior, System.ExecutionEngineException on System.Net.Http related code.

joperezr · 2018-03-28T20:40:00Z

We actually did get the fix in to 15.6.3 (which we already shipped), so if you install the latest RTM product this should be there. Do you see the file C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\Microsoft.NET.Build.Extensions\net471\lib\netfx.force.conflicts.dll in your machine? If so then it means the fix is there and what you are seeing is caused by something else. If that is the case, do you mind sharing a small repro or a build log so that I can take a look what is going on?

mscrivo · 2018-03-28T20:45:41Z

@joperezr I do have that file in both my 15.7 Preview folder and 15.6.4 folder. Interesting! I will try to get you a build log and a minimal repro solution. Is there someway less public that I can send you a build log and perhaps a memory dump from IIS?

mscrivo · 2018-03-29T02:24:50Z

@joperezr I created a sample app that repro's the issue somewhat: https://github.com/mscrivo/System.Net.Http.Crash

I say somewhat, because it seems the root cause is the same, but in our real app, we end up with a crash instead, but anyhow, I think this should be sufficient to show the problem.

The app at this repo consists of a .NET Standard 2.0 lib that has a generic API client which uses HttpClient and a standard ASP.NET MVC project (using .NET 4.7.1). The Web project references the .net standard project and uses the generic API Client from the netstandard project in a class. It then uses SimpleInjector to wire up those implementations and the generic API client implementations in the Global.asax. This solution will build without any errors or warnings, but if you run it within IIS, you'll get the following:

[MissingMethodException: Method not found: 'Void NetStandardLib.ApiClient`1..ctor(System.Net.Http.HttpClient, Newtonsoft.Json.JsonSerializer, System.Version)'.]
   WebApplication1.LinkedInServiceFactory..cctor() in C:\Code\System.Net.Http.Crash\WebApplication1\SampleApiClient.cs:16

[TypeInitializationException: The type initializer for 'WebApplication1.LinkedInServiceFactory' threw an exception.]
   WebApplication1.LinkedInServiceFactory..ctor() +0
   lambda_method(Closure ) +72
   SimpleInjector.Lifestyles.SingletonLifestyleRegistration`1.CreateInstanceWithNullCheck() +108
   SimpleInjector.Lifestyles.SingletonLifestyleRegistration`1.GetInterceptedInstance() +100
   SimpleInjector.Lifestyles.SingletonLifestyleRegistration`1.BuildExpression() +17
   SimpleInjector.InstanceProducer.BuildExpressionInternal() +46
   System.Lazy`1.CreateValue() +727
   System.Lazy`1.LazyInitValue() +184
   SimpleInjector.InstanceProducer.BuildExpression() +68

[ActivationException: The type initializer for 'WebApplication1.LinkedInServiceFactory' threw an exception.]
   SimpleInjector.InstanceProducer.BuildExpression() +285
   SimpleInjector.InstanceProducer.VerifyExpressionBuilding() +29

on the following line:
private static readonly IApiClient<LinkedInError> ApiClient = new ApiClient<LinkedInError>(CreateHttpClient(), Serializer, HttpVersion.Version10);

mscrivo · 2018-03-29T13:50:49Z

The sample solution does in fact repro the ExecutionEngineException problem as well, you just have to rebuild a few times and reload the app in IIS and you'll get it on this line: container.Register(typeof(IApiClient<>), typeof(ApiClient<>), Lifestyle.Singleton);

joperezr · 2018-03-29T16:46:13Z

ok, thanks for the repro I'll take a look at it today.

mscrivo · 2018-03-29T16:50:49Z

Thank you

joperezr · 2018-03-29T17:39:50Z

@mscrivo I see what's going on. Apparently, this seems to be a known issue we have I just don't know if we actually have an issue on GitHub for it. The problem is with our tooling and it is specific to ASP.NET Web Applications. First of all, because of the way they are built, automatic binding redirects generation is currently broken. That basically forces you to do what you already did, which is to double-click the warning in VS in order to add those to your Web.config. Secondly, once the app runs in IIS, the extra dlls that we add to your bin directory won't get copied over, so that will make the app fail at runtime since it won't be able to find System.Net.Http 4.2.0.0 version. A tooling team is currently working on fixing both of these issues but unfortunately, we don't have an ETA for it yet.

@terrajobst are there any known workarounds for this issue? For example, some target that will deploy the extra runtime facades into the IIS folder?

mscrivo · 2018-03-29T17:48:52Z

Thanks @joperezr .. I'm frankly surprised more people haven't run into this. A better workaround would be nice as our current one leads to many build warnings regarding the System.Net.Http binding redirects being wrong (since we are forcing 4.0), and intellisense broken in VS whenever it comes to code that deals with HttpClient.

joperezr · 2018-03-29T17:50:44Z

I totally agree with you. I'll make sure to push for a fix ASAP from our tooling side.

jwisener · 2018-03-29T21:24:00Z

We are running into the same issues. I am having to copy the shim dlls and include them into our source control and manually set a reference and tell it to copy, until a fix is in place. This has been very frustrating, especially when we have deadlines to deliver business value to our end customers.

jwisener · 2018-03-29T21:29:57Z

I don’t understand why you just couldnt have kept the version number the same as the framework, the public key was left the same. This reminds me of the days of COM and DLL hell.

kylef000 · 2018-06-11T14:25:04Z

@joperezr

Was the issue mentioned here #1712 (comment) ever resolved? Still running into this with VS2017 15.7.3 and an ASP .NET targeting 4.7.1.

joperezr · 2018-06-11T17:10:18Z

I don't believe there is a tooling fix for that yet, but we fixed the underlining issue requiring those extra shims so that if you now target .NET Framework 4.7.2 you shouldn't see this issues any longer.

kylef000 · 2018-06-11T17:57:27Z

@joperezr Thanks for the speedy response! Sounds good, I'll retarget for 4.7.2.

eriawan · 2018-08-16T04:17:29Z

I have tested the 15.8.0, using .NET Framework 4.7.2 as the target.

It is fixed! 👍

…902.1 (#1712) [main] Update dependencies from dotnet/arcade

Changes to the msbuild extensions targets that will enable copying th…

7cd66f1

…e shims for net471 case

AlexGhiondea reviewed Nov 7, 2017

View reviewed changes

joperezr force-pushed the AddNet471Shims branch from 1912bdf to 9fc5e5a Compare November 8, 2017 17:38

Adding tests and updating the reference to the support package

bf8f996

joperezr force-pushed the AddNet471Shims branch from 9fc5e5a to bf8f996 Compare November 8, 2017 17:52

joperezr commented Nov 8, 2017

View reviewed changes

dsplaisted approved these changes Nov 8, 2017

View reviewed changes

nguerrera approved these changes Nov 8, 2017

View reviewed changes

Fixing comments on targets file and adding a test case

d20f5c7

dsplaisted reviewed Nov 8, 2017

View reviewed changes

One more change to a comment to make it more descriptive

e3adf53

dsplaisted mentioned this pull request Nov 8, 2017

Include updated Http and Compression libraries for .NET 4.7.1 if necessary #1667

Closed

dsplaisted merged commit 6c6a6bb into dotnet:release/15.5 Nov 8, 2017

mscrivo mentioned this pull request Feb 7, 2018

ExecutionEngineException in RegisterWebApiControllers simpleinjector/SimpleInjector#508

Closed

JL03-Yue pushed a commit that referenced this pull request Mar 19, 2024

Update dependencies from https://github.com/dotnet/arcade build 20220…

e2f247d

…902.1 (#1712) [main] Update dependencies from dotnet/arcade

joperezr deleted the AddNet471Shims branch June 12, 2024 20:21

Changes to the msbuild extensions targets that will enable copying the shims for net471 case #1712

Changes to the msbuild extensions targets that will enable copying the shims for net471 case #1712

Conversation

joperezr commented Nov 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexGhiondea Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mscrivo commented Feb 7, 2018 • edited Loading

joperezr commented Feb 7, 2018

mscrivo commented Feb 7, 2018

joperezr commented Feb 7, 2018

eriawan commented Feb 12, 2018 • edited Loading

mscrivo commented Mar 6, 2018

joperezr commented Mar 6, 2018

mscrivo commented Mar 6, 2018

mscrivo commented Mar 28, 2018

joperezr commented Mar 28, 2018

mscrivo commented Mar 28, 2018 • edited Loading

mscrivo commented Mar 29, 2018

mscrivo commented Mar 29, 2018

joperezr commented Mar 29, 2018

mscrivo commented Mar 29, 2018

joperezr commented Mar 29, 2018

mscrivo commented Mar 29, 2018

joperezr commented Mar 29, 2018

jwisener commented Mar 29, 2018

jwisener commented Mar 29, 2018

kylef000 commented Jun 11, 2018

joperezr commented Jun 11, 2018

kylef000 commented Jun 11, 2018

eriawan commented Aug 16, 2018

AlexGhiondea Nov 7, 2017 •

edited

Loading

mscrivo commented Feb 7, 2018 •

edited

Loading

eriawan commented Feb 12, 2018 •

edited

Loading

mscrivo commented Mar 28, 2018 •

edited

Loading