-
Notifications
You must be signed in to change notification settings - Fork 560
[xabt] Compute $DOTNET_DiagnosticPorts
using MSBuild properties
#10351
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
Conversation
Because of the issues with escaping
Going to turn this into multiple properties to match the design we came up with here: |
If you run `dotnet-trace collect --dsrouter android`, it says: Start an application on android device with ONE of the following environment variables set: [Default Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,nosuspend,connect [Startup Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,suspend,connect Setting `$DOTNET_DiagnosticPorts` is non-trivial, so as a step to simplify this, we are adding multiple MSBuild properties to configure this value. This would allow the log message to say: Build and run an Android application such as: [Default Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=false -p:DiagnosticListenMode=connect [Startup Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=true -p:DiagnosticListenMode=connect You could also set `$(DiagnosticConfiguration)`, but you would need to escape the `,` character with `%2c`. Setting any of the new properties also implicitly means that `$(AndroidEnableProfiler)` is set to `true`. I updated a couple tests to verify the new properties work.
af1a622
to
4c20250
Compare
$(DiagnosticPorts)
for dotnet-dsrouter
$DOTNET_DiagnosticPorts
using MSBuild properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
<ItemGroup> | ||
<_GeneratedAndroidEnvironment Include="mono.enable_assembly_preload=0" Condition=" '$(AndroidEnablePreloadAssemblies)' != 'True' " /> | ||
<_GeneratedAndroidEnvironment Include="DOTNET_MODIFIABLE_ASSEMBLIES=Debug" Condition=" '$(AndroidIncludeDebugSymbols)' == 'true' and '$(AndroidUseInterpreter)' == 'true' " /> | ||
<_GeneratedAndroidEnvironment Include="DOTNET_DiagnosticPorts=$(DiagnosticConfiguration)" Condition=" '$(DiagnosticConfiguration)' != '' " /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any risk this might get stuck in the build or would a new build and/or run without any Diagnostic* properties set remove all diagnostic support in the build making sure this part of the build gets updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part looks OK for incremental builds, this target has no Inputs/Outputs, so it always runs.
Then <WriteLinesToFile/>
uses WriteOnlyWhenDifferent="True"
, which would update the timestamp on the @(AndroidEnvironment)
file it writes if changed.
Later on, it looks like all the important targets will rerun if @(AndroidEnvironment)
files change.
…tnet#10351) If you run `dotnet-trace collect --dsrouter android`, it says: Start an application on android device with ONE of the following environment variables set: [Default Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,nosuspend,connect [Startup Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,suspend,connect Setting `$DOTNET_DiagnosticPorts` is non-trivial, so as a step to simplify this, we are adding multiple MSBuild properties to configure this value. This would allow the log message to say: Build and run an Android application such as: [Default Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=false -p:DiagnosticListenMode=connect [Startup Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=true -p:DiagnosticListenMode=connect You could also set `$(DiagnosticConfiguration)`, but you would need to escape the `,` character with `%2c`. Setting any of the new properties also implicitly means that `$(AndroidEnableProfiler)` is set to `true`.
Context: dotnet/android#10351 Context: dotnet/macios#23429 We are adding new MSBuild properties to simplify setting `$DOTNET_DiagnosticPorts` on mobile: * `$(DiagnosticAddress)` * `$(DiagnosticPort)` * `$(DiagnosticSuspend)` * `$(DiagnosticListenMode)` * `$(DiagnosticConfiguration)` if you want to specify the full value yourself, escape `,` with `%2c`, etc. We will ship these properties in future releases of .NET 9 and 10. To improve `dsrouter` the current log message: Start an application on android device with ONE of the following environment variables set: [Default Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,nosuspend,connect [Startup Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,suspend,connect Will change to: Build and run an Android application such as: [Default Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=false -p:DiagnosticListenMode=connect [Startup Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=true -p:DiagnosticListenMode=connect Note that `dotnet run` *does work*, but it doesn't show good progress on the build & deploy steps as compared to `dotnet build -t:Run` and MSBuild's terminal logger. It can take several seconds to run a `Release` build. I think it's better to recommend `-t:Run` until we improve `dotnet run`.
…0351) If you run `dotnet-trace collect --dsrouter android`, it says: Start an application on android device with ONE of the following environment variables set: [Default Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,nosuspend,connect [Startup Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,suspend,connect Setting `$DOTNET_DiagnosticPorts` is non-trivial, so as a step to simplify this, we are adding multiple MSBuild properties to configure this value. This would allow the log message to say: Build and run an Android application such as: [Default Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=false -p:DiagnosticListenMode=connect [Startup Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=true -p:DiagnosticListenMode=connect You could also set `$(DiagnosticConfiguration)`, but you would need to escape the `,` character with `%2c`. Setting any of the new properties also implicitly means that `$(AndroidEnableProfiler)` is set to `true`.
…0351) If you run `dotnet-trace collect --dsrouter android`, it says: Start an application on android device with ONE of the following environment variables set: [Default Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,nosuspend,connect [Startup Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,suspend,connect Setting `$DOTNET_DiagnosticPorts` is non-trivial, so as a step to simplify this, we are adding multiple MSBuild properties to configure this value. This would allow the log message to say: Build and run an Android application such as: [Default Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=false -p:DiagnosticListenMode=connect [Startup Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=true -p:DiagnosticListenMode=connect You could also set `$(DiagnosticConfiguration)`, but you would need to escape the `,` character with `%2c`. Setting any of the new properties also implicitly means that `$(AndroidEnableProfiler)` is set to `true`.
Context: dotnet/android#10351 Context: dotnet/macios#23429 We are adding new MSBuild properties to simplify setting `$DOTNET_DiagnosticPorts` on mobile: * `$(DiagnosticAddress)` * `$(DiagnosticPort)` * `$(DiagnosticSuspend)` * `$(DiagnosticListenMode)` * `$(DiagnosticConfiguration)` if you want to specify the full value yourself, escape `,` with `%2c`, etc. We will ship these properties in future releases of .NET 9 and 10. To improve `dsrouter` the current log message: Start an application on android device with ONE of the following environment variables set: [Default Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,nosuspend,connect [Startup Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,suspend,connect Will change to: Build and run an Android application such as: [Default Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=false -p:DiagnosticListenMode=connect [Startup Tracing] dotnet build -t:Run -c Release -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=true -p:DiagnosticListenMode=connect Note that `dotnet run` *does work*, but it doesn't show good progress on the build & deploy steps as compared to `dotnet build -t:Run` and MSBuild's terminal logger. It can take several seconds to run a `Release` build. I think it's better to recommend `-t:Run` until we improve `dotnet run`.
…23429) If you run `dotnet-dsrouter ios`, it says: Start an application on ios device with ONE of the following environment variables set: [Default Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,nosuspend,listen [Startup Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,suspend,listen Setting `$DOTNET_DiagnosticPorts` is non-trivial, so as a step to simplify this, we are adding multiple MSBuild properties to configure this value. This would allow the log message to say: Build and run an iOS application such as: [Default Tracing] dotnet run -c Release -p:ios-arm64 -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=false -p:DiagnosticListenMode=listen [Startup Tracing] dotnet run -c Release -p:ios-arm64 -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=true -p:DiagnosticListenMode=listen Since these are all default values, it can be simplified to: Build and run an iOS application such as: [Default Tracing] dotnet run -p:ios-arm64 -c Release -p:EnableDiagnostics=true [Startup Tracing] dotnet run -p:ios-arm64 -c Release -p:EnableDiagnostics=true Setting any of the new properties also implicitly means that `$(EnableDiagnostics)` is set to `true` (first example), or alternatively `$(EnableDiagnostics)` can be set to true to get the default value for all the new properties (second example). Ref: dotnet/android#10351 --------- Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
…ild properties. (#23629) If you run `dotnet-dsrouter ios`, it says: Start an application on ios device with ONE of the following environment variables set: [Default Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,nosuspend,listen [Startup Tracing] DOTNET_DiagnosticPorts=127.0.0.1:9000,suspend,listen Setting `$DOTNET_DiagnosticPorts` is non-trivial, so as a step to simplify this, we are adding multiple MSBuild properties to configure this value. This would allow the log message to say: Build and run an iOS application such as: [Default Tracing] dotnet run -c Release -p:ios-arm64 -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=false -p:DiagnosticListenMode=listen [Startup Tracing] dotnet run -c Release -p:ios-arm64 -p:DiagnosticAddress=127.0.0.1 -p:DiagnosticPort=9000 -p:DiagnosticSuspend=true -p:DiagnosticListenMode=listen Since these are all default values, it can be simplified to: Build and run an iOS application such as: [Default Tracing] dotnet run -p:ios-arm64 -c Release -p:EnableDiagnostics=true [Startup Tracing] dotnet run -p:ios-arm64 -c Release -p:EnableDiagnostics=true Setting any of the new properties also implicitly means that `$(EnableDiagnostics)` is set to `true` (first example), or alternatively `$(EnableDiagnostics)` can be set to true to get the default value for all the new properties (second example). Ref: dotnet/android#10351 Backport of #23429. --------- Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
If you run
dotnet-trace collect --dsrouter android
, it says:Setting
$DOTNET_DiagnosticPorts
is non-trivial, so as a step tosimplify this, we are adding multiple MSBuild properties to configure
this value.
This would allow the log message to say:
You could also set
$(DiagnosticConfiguration)
, but you would need toescape the
,
character with%2c
.Setting any of the new properties also implicitly means that
$(AndroidEnableProfiler)
is set totrue
.I updated a couple tests to verify the new properties work.