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

Invoke intrinsic commands directly instead of creating a process. #1208

Merged
merged 1 commit into from Feb 4, 2016

Conversation

Projects
None yet
3 participants
@AustinWise
Contributor

AustinWise commented Feb 3, 2016

This is a second try of pull request #1197.

Since issue #1149 intrinsic commands are now invoked directly from the dotnet.exe program rather than creating a new a new dotnet-intrinsic.exe process. Now that all the commands live inside the same assembly, intrinsic commands that invoke other commands could potentially do so without creating a new process. As creating a process, loading the CLR, resolving dependencies, and JIT-ing code all take a bit of time, this change could speed things up a bit.

I believe the principal change in behavior this change causes is for the DOTNET_CLI_CONTEXT_ environmental variables to be inherit by intrinsic commands created by other intrinsic commands. Currently Microsoft.DotNet.Cli.Program.ProcessArgs() seems to overwrite this environmental variable every time based on command line flags.

There are probably nicer ways of doing this, but I wanted to try the simplest thing possible to see if it would work and if y'all are interested in doing something like this.

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Feb 3, 2016

Hi @AustinWise, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

dnfclas commented Feb 3, 2016

Hi @AustinWise, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@davidfowl

This comment has been minimized.

Show comment
Hide comment
@davidfowl

davidfowl Feb 4, 2016

Collaborator

Man I wish I knew why this is un-mergable. We want to take this change though..

Collaborator

davidfowl commented Feb 4, 2016

Man I wish I knew why this is un-mergable. We want to take this change though..

@AustinWise

This comment has been minimized.

Show comment
Hide comment
@AustinWise

AustinWise Feb 4, 2016

Contributor

I rebased this change onto the tip of the 1.0.0 branch and solved the merge conflict. Fingers crossed that it does not become unmergable too quickly.

Contributor

AustinWise commented Feb 4, 2016

I rebased this change onto the tip of the 1.0.0 branch and solved the merge conflict. Fingers crossed that it does not become unmergable too quickly.

davidfowl added a commit that referenced this pull request Feb 4, 2016

Merge pull request #1208 from AustinWise/intrinsicInvoke
Invoke intrinsic commands directly instead of creating a process.

@davidfowl davidfowl merged commit d1b8eb9 into dotnet:rel/1.0.0 Feb 4, 2016

8 checks passed

CentOS7.1 Debug Build Build finished. 32 tests run, 0 skipped, 0 failed.
Details
CentOS7.1 Release Build Build finished. 30 tests run, 0 skipped, 0 failed.
Details
OSX Debug Build Build finished. 32 tests run, 0 skipped, 0 failed.
Details
OSX Release Build Build finished. 30 tests run, 0 skipped, 0 failed.
Details
Ubuntu Debug Build Build finished. 32 tests run, 0 skipped, 0 failed.
Details
Ubuntu Release Build Build finished. 30 tests run, 0 skipped, 0 failed.
Details
Windows_NT Debug Build Build finished. 68 tests run, 2 skipped, 0 failed.
Details
Windows_NT Release Build Build finished. 30 tests run, 0 skipped, 0 failed.
Details

@AustinWise AustinWise deleted the AustinWise:intrinsicInvoke branch Feb 4, 2016

wli3 pushed a commit to wli3/cli that referenced this pull request Jul 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment