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

Add Microsoft.DotNet.RemoteExecutor #2176

Merged
merged 12 commits into from
Apr 2, 2019

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Mar 5, 2019

Removed all corefx dependencies. UseRemoteExecutor will be set in the test projects that use the RemoteExecutor.

@ViktorHofer ViktorHofer self-assigned this Mar 5, 2019
@ViktorHofer ViktorHofer requested a review from ericstj March 5, 2019 22:59
@ericstj ericstj requested review from safern and wtgodbe March 27, 2019 16:48
@ViktorHofer
Copy link
Member Author

@safern @ericstj @wtgodbe PTAL


<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netstandard2.0</TargetFramework>
Copy link
Member

Choose a reason for hiding this comment

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

App targeting netstandard2.0 then hacking in a runtime.config for netcoreapp2.0, why not just target netcoreapp2.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to explain it but then my browser crashed -,-. Let's talk about it in the standup.

Copy link
Member

Choose a reason for hiding this comment

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

Well , if you expect us to accept your position just go ahead and commit it to the source file as a comment. In general this is a bad idea since we don't have any sort of cross-framework app launcher, nor a loader that can multiplex framework specific dependencies.

Copy link
Member Author

@ViktorHofer ViktorHofer Apr 1, 2019

Choose a reason for hiding this comment

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

So the straight answer here is that if I would change the TargetFramework in Microsoft.DotNet.RemoteExecutorHost to netcoreapp2.0 I wouldn't be able to reference it from the Microsoft.DotNet.RemoteExecutor which targets netstandard2.0.

The long answer is that I tried different ways to cross-compile both Microsoft.DotNet.RemoteExecutor and Microsoft.DotNet.RemoteExecutorHost for netcoreapp, netfx and uap / netstandard. The problem was that the test projects in corefx that target netstandard can't reference the netcoreapp/ netfx / uap implementation even though they run on that framework at the end of the day. In example when I compiled against netcoreapp2.0 the version of System.Runtime was 4.2.0.0 which caused a conflict with the netstandard version of System.Runtime 4.1.2.0.

I decided that targeting netstandard2.0 is fine for Microsoft.DotNet.RemoteExecutor and Microsoft.DotNet.RemoteExecutorHost as a) runtime checks instead of different cross-compiled versions aren't costly and work fine today, b) The invocation of the RemoteExecutorHost is controlled by the Microsoft.DotNet.RemoteExecutor class library, means it supports invoking the netstandard executable depending on the current runtime, i.e. netfx invokes the dll directly, netcoreapp invokes the dll with the dotnet host and a corresponding runtimeconfig.json file.

The default runtimeconfig.json is just to set a minimum netcoreapp supported version + a major version upgrade policy. If you want to pinpoint it to a specific shared framework you usually want to override that runtimeconfig.json file. In corefx we create custom runtimeconfig.json and .dll.config files to point to our shared framework / netfx runtime.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Apr 2, 2019

@eerhardt @Anipik @safern not sure if ML.NET needs remote execution at the moment but just in case, this package (netstandard2.0) is referenceable by the test projects and can simply invoke remote code like in corefx:

using Microsoft.DotNet.RemoteExecutor;

RemoteExecutor.Invoke(...);

I remember that someone asked for a remote execution functionality in ML.NET at least in the past.

@Anipik
Copy link
Contributor

Anipik commented Apr 2, 2019

yes ML repo is using remote executor for testing some of the scenarios. It was added in dotnet/machinelearning#2200
I can throw up a PR to use this new package and delete that code

Arcade.sln Show resolved Hide resolved
@ViktorHofer
Copy link
Member Author

I can throw up a PR to use this new package and delete that code

Yes I think that's the right thing to do. Can you please create an issue to track this?

@Anipik
Copy link
Contributor

Anipik commented Apr 2, 2019

Yes I think that's the right thing to do. Can you please create an issue to track this?

Yeah sure, i will create that.

@ViktorHofer ViktorHofer merged commit 505e389 into dotnet:master Apr 2, 2019
@ViktorHofer ViktorHofer deleted the RemoteExecutor branch April 2, 2019 00:48
chcosta added a commit to chcosta/arcade that referenced this pull request Apr 8, 2019
Updates (dotnet#2361)

Update dependencies from https://github.com/dotnet/arcade build 20190327.11 (dotnet#2363)

- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19177.11
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19177.11
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19177.11
- Microsoft.DotNet.SignTool - 1.0.0-beta.19177.11
- Microsoft.DotNet.SwaggerGenerator.MSBuild - 1.0.0-beta.19177.11

Add missing corefx test files for mono

Update JenkinsShutdownStatus.md

Update JenkinsShutdownStatus.md

Update JenkinsShutdownStatus.md

Move images folder to correct subfolder (dotnet#2372)

Added partial facade generator (dotnet#2221)

* added partial facade genertor

* c# coding done

* making corefx projects work with new genfacades

* removing nesting types from reference assembly, adding generic support nad increasing restriction of public types

* Minor naming changes and other minor feedback

* making nested, full facades and some dwarning disables

* using ispartialfacade to use new tool

* making compile errors work

* Added missingtypes List

* eric feedback

* name changes, catch exception removed and spport for generic type parameters greater than 9

* GenFacadesOmitType added and IgnoreMissitypeList prefixed with genfacades

* avoiding doube casting, omittype name change

WIP

Create a GH issue on publishing error(s)

Try to tag non-bot accounts only

Make a NuGet package instead

Remove Octo using

Correct passed values to .proj

Correct passed values to .proj

Revert changes in .proj

Just create the package

PR feedback

Add envs for artifacts, toolset, tools, and log directories

fixes dotnet#2276

use VSO vars instead

remove spaces between ] and $

Update NGEN optimization doc (dotnet#2355)

Add support for signing validation in the publishing release pipelines. (dotnet#2362)

* Add support for signing validation in the publishing release pipelines.

* Some adjustments

* More comments.

* PR feedback: add --traverse-subfolders

Fix typos in ApiCompat project (dotnet#2293)

* Fix typos in output text

* Fix typos in comments

Update dependencies from https://github.com/dotnet/arcade build 20190329.1 (dotnet#2380)

- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19179.1
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19179.1
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19179.1
- Microsoft.DotNet.SignTool - 1.0.0-beta.19179.1
- Microsoft.DotNet.SwaggerGenerator.MSBuild - 1.0.0-beta.19179.1

Update JenkinsShutdownStatus.md

Fix race with AssemblyLoadContext.ResolveUsingResolvingEvent (dotnet#2360)

Remove xunit.console.deps.json from output dir (dotnet#2381)

2286: Return passing result test for exit code 0 when no result files are found.

Fix NGEN method logging

The NGEN method logging code didn't account for the fact that we run
IBCMerge for the same assembly multiple times when the project uses
multi-targeting. This created race conditions on both writing the temp
XML file and the NGEN method list.

This PR fixes this by properly writing out separate files for each
target framework of an assembly.

closes dotnet#2309

Remove MVID

After verifying the race locally removing the MVID. It's clear the race
is about TF which is now properly covered. Keeping the MVID out of the
name means that we can more easily diff NGEN between builds.

Respond to PR feedback

Use verbose for xunit execution

By default the xunit output only includes tests which were skipped or
failed. It provides no information about how tests are executed.

This is fine for day to day execution but insufficient for CI jobs. When
tests crash rudely then there is no context for why a test failed. For
instance if there is a `StackOverflowException` or sigsegv then the log
doesn't have any data indicating what test failed.

The `-verbose` flag causes xunit to output detailed information about
test execution including Starting and Finished notations. With this
output its clear what test caused the crash as there will be a Starting
notation as the last entry in the log.

This can be invaluable when tracking down crashes / hangs. Particularly
on Unix where we don't have dump support yet.

Example output for a successfully executing test.

```
Microsoft.CodeAnalysis.CSharp.UnitTests.PreprocessorTests.TestIfFalseElifFalseElseEndif [STARTING]
Microsoft.CodeAnalysis.CSharp.UnitTests.PreprocessorTests.TestIfFalseElifFalseElseEndif [FINISHED] Time: 0.0003478s
```

Update dependencies from https://github.com/dotnet/arcade build 20190329.2 (dotnet#2387)

- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19179.2
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19179.2
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19179.2
- Microsoft.DotNet.SignTool - 1.0.0-beta.19179.2
- Microsoft.DotNet.SwaggerGenerator.MSBuild - 1.0.0-beta.19179.2

Update JenkinsShutdownStatus.md

Update JenkinsShutdownStatus.md

Add angular language to swagger generator (dotnet#2378)

Fix docs URL reference

Fix docs URL reference

add append line in sourcegenerator to comment that the following is autogenerated

Add xunit.console.deps.json only for netcoreapp (dotnet#2388)

Make project packable

Add Microsoft.DotNet.RemoteExecutor (dotnet#2176)

* Add Microsoft.DotNet.RemoteExecutor

Update dependencies from https://github.com/dotnet/arcade build 20190401.12 (dotnet#2400)

- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19201.12
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19201.12
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19201.12
- Microsoft.DotNet.SignTool - 1.0.0-beta.19201.12
- Microsoft.DotNet.SwaggerGenerator.MSBuild - 1.0.0-beta.19201.12

Remove deps from test archive (dotnet#2399)

Update JenkinsShutdownStatus.md

Fix a typo in StartHere.md (dotnet#2325)

Fix exit codes (dotnet#2394)

It used to be that a return code of 2 was success (indicating the client was disconnected).  Now, 0 appears to be the correct return code.

Addressing issues with the new GenfacadesSourceTask (dotnet#2404)

* moving using task to main file for shims, adding license header, exposing assembly path

* handing platformNotSupportedGeneratedCode

* Running GenerateNotSupportedSource as first target of coreCompileDependsOn, hence removing the dependency of genfacades

Use reference Id for enum names if it exists. (dotnet#2405)

Start consuming the latest portable IBCMerge (dotnet#2365)

Update dependencies from https://github.com/dotnet/arcade build 20190402.13 (dotnet#2410)

- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19202.13
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19202.13
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19202.13
- Microsoft.DotNet.SignTool - 1.0.0-beta.19202.13
- Microsoft.DotNet.SwaggerGenerator.MSBuild - 1.0.0-beta.19202.13

missing property Tag (dotnet#2414)

[Pool Provider] Prod, more logging, and correlation script (dotnet#2409)

* Fix exit codes
It used to be that a return code of 2 was success (indicating the client was disconnected).  Now, 0 appears to be the correct return code.

* [Pool Provider] Prod, more logging, and correlation script
- Add a prod deployment (which is really ahead of int at this point).
- Add a additional logging in based on the Maestro logging setup
- Add a rough script that can map from a build onto the helix work items (and other info) that was submitted for that build

Move TheoryData helper to XUnitExtensions (dotnet#2403)

* Move TheoryData helper to XUnitExtensions

* Move tests over to XUnitExtensions

Add support for junit results (dotnet#2421)

* Add support for junit results

This changes adds support to the helix sdk azure pipelines reporter to process junit xml results from junit-results.xml files.

* skip_reason can't be none

* Use if instead of or because of FutureWarning

* Add validation of expected failures

* Use correct property name

* Make expected failed tests correct.

Update dependencies from https://github.com/dotnet/arcade build 20190403.10 (dotnet#2425)

- Microsoft.DotNet.Arcade.Sdk - 1.0.0-beta.19203.10
- Microsoft.DotNet.Build.Tasks.Feed - 2.2.0-beta.19203.10
- Microsoft.DotNet.Helix.Sdk - 2.0.0-beta.19203.10
- Microsoft.DotNet.SignTool - 1.0.0-beta.19203.10
- Microsoft.DotNet.SwaggerGenerator.MSBuild - 1.0.0-beta.19203.10

Update AsyncPublishing_HowToUse.md (dotnet#2424)

Adding other repo feeds

Publish symbols using release pipelines (dotnet#2407)

* Adding PublishToSymbolServers

Fix itemgroup adding

Fix filename

Fix bad merge

Fix bad merge

remove verbose
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.

3 participants