Add environment variables for Launcher path and args in ANCM #19797
Conversation
BrennanConroy
requested review from
analogrelay,
halter73 and
Tratcher
as code owners
| { | ||
| var deploymentParameters = Fixture.GetBaseDeploymentParameters(hostingModel); | ||
|
|
||
| deploymentParameters.EnvironmentVariables["ANCM_LAUNCHER_PATH"] = "nope"; |
There was a problem hiding this comment.
@jkotalik I'm hoping you might have some ideas of how to write some positive tests instead of just negative 😄
There was a problem hiding this comment.
You could copy the app to a different temporary folder and set ANCM_LAUNCHER_PATH/LAUNCHER_ARGS to that folder?
There was a problem hiding this comment.
Shouldn't need to copy anything, but how do I know where the app is located? Ah, you can do a publish to get the folder of an app and then set the variables to that folder and then run deploy. And is launcher path always just dotnet.exe? Because I have _dotnetLocation available.
|
@BrennanConroy if you can provide either an MSI or just a share with the changed DLLs to @BillHiebert, I expect he can start looking at the matching VS changes. |
|
I'm assuming he'll want both changes in the DLLs? |
| @@ -101,6 +102,8 @@ REQUESTHANDLER_CONFIG::Populate( | |||
| BSTR bstrBasicAuthSection = NULL; | |||
| BSTR bstrAnonymousAuthSection = NULL; | |||
| BSTR bstrAspNetCoreSection = NULL; | |||
| auto launcherPathEnv = Environment::GetEnvironmentVariableValue(CS_ANCM_LAUNCHER_PATH); | |||
| auto launcherArgsEnv = Environment::GetEnvironmentVariableValue(CS_ANCM_LAUNCHER_ARGS); | |||
There was a problem hiding this comment.
We need to be careful about calling GetEnvironmentVariableValue in the out of proc code. For example, if GetEnvironmentVariableValue fails, it throws an exception. I'd recommend try/catching around these call or checking where the next try catch is.
src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp
Outdated
Show resolved
Hide resolved
Yep. I was thinking that because the other change had merged it would be relatively straightforward to get both changes into a build now. If this PR moves quickly enough we can wait for it to merge, I just don't want us to wait too long here :). |
BrennanConroy
force-pushed
the
brecon/launcher
branch
from
7312db1 to
a78720e
Compare
amcasey
added
area-networking
Fixes #19451