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.

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 value tuple #4950

Merged
merged 3 commits into from
Sep 15, 2017
Merged

remove value tuple #4950

merged 3 commits into from
Sep 15, 2017

Conversation

SimonCropp
Copy link
Contributor

No description provided.

@SimonCropp SimonCropp requested a review from a team September 13, 2017 11:58
@danielmarbach
Copy link
Contributor

This is great. I had huge assembly redirect problems today because of ValueTuple. See

dotnet/standard#476
SixLabors/ImageSharp#270

and many more

@danielmarbach
Copy link
Contributor

What is interesting is the following, although I added

        var scanner = configuration.AssemblyScanner();
        scanner.ExcludeAssemblies("System.ValueTuple.dll");

and

    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.ValueTuple" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-4.0.2.0" newVersion="4.0.2.0" />
      </dependentAssembly>
    </assemblyBinding>

it is still failing with a BadImageFormatException

   at NServiceBus.Hosting.Helpers.AssemblyScanner.TryLoadScannableAssembly(String assemblyPath, AssemblyScannerResults results, Assembly& assembly)
   at NServiceBus.Hosting.Helpers.AssemblyScanner.GetScannableAssemblies()
   at NServiceBus.EndpointConfiguration.GetAllowedTypes(String path)
   at NServiceBus.EndpointConfiguration.Build()
   at NServiceBus.Endpoint.Create(EndpointConfiguration configuration)
   at NServiceBus.Endpoint.<Start>d__1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at WorkerRole.<StartEndpoint>d__3.MoveNext() in C:\p\docs.particular.net\samples\azure\self-host\Core_7\HostWorker\WorkerRole.cs:line 61
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at WorkerRole.OnStart() in C:\p\docs.particular.net\samples\azure\self-host\Core_7\HostWorker\WorkerRole.cs:line 17
   at Microsoft.WindowsAzure.ServiceRuntime.RoleEnvironment.InitializeRoleInternal(RoleType roleTypeEnum)
   at Microsoft.WindowsAzure.ServiceRuntime.Implementation.Loader.RoleRuntimeBridge.<InitializeRole>b__0()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

@danielmarbach
Copy link
Contributor

danielmarbach commented Sep 13, 2017

As an FYI it turned out the problem manifestation is a combination of Microsoft.WindowsAzure.targets with old csproj using PackageReference with a ref assembly. More details can be found Azure/azure-sdk-for-net#3699. Thanks to @bording for helping me out with the investigation on this one!

andreasohlund
andreasohlund previously approved these changes Sep 14, 2017
@andreasohlund
Copy link
Member

Even though it was a problem with the azure target this highlights that this can cause issues that we can't forsee. Given that this has no external impact and we can start using it as soon as we bump to net471 (?) I'd say the risk isn't worth it.

Lets get this in

@dbelcham
Copy link
Contributor

@SimonCropp we're looking at making this change, but we need this PR to compile and pass tests first.

@SimonCropp
Copy link
Contributor Author

build is fixed. please merge

@andreasohlund andreasohlund merged commit 00fe48d into develop Sep 15, 2017
@andreasohlund andreasohlund deleted the valueTupleAgain branch September 15, 2017 06:16
@andreasohlund
Copy link
Member

I will remove the upgrade note about this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants