This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
Remove run.exe and config.json #21608
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This simulates the behavior of Tools\msbuild.cmd
Previously, config.json had its own processing that would set OSName. Instead, we now pass it in explicitly where it's needed (building packages), or not at all (publishing them).
This seems necessary to properly handle the azure access token
|
This is also worth considering if we still rely on old dependencies flowing https://github.com/dotnet/versions/blob/59a50129040abf769dc33a60ff4a36ec74c813ba/Maestro/subscriptions.json#L204 as well as the runtests scripts. That was the main reasons I didn't rip it out. But this also sounds good to start transitioning. |
Required for cases where the build OS isn't detected during the build such as freebsd.
Similarly to publish-packages.cmd, sync.cmd needs to be able to parse an access token that contains '=' correctly.
|
Thanks @hoyosjs, I'll update the subscription once this is in. |
|
Successful buildpipeline build with these changes: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=2322642 |
jashook
approved these changes
Jan 11, 2019
|
Also in Line 1587 in 8a15c6b |
This prevents us from having to deal with different quote escape behavior on windows and linux. Previously, arguments like fileloggerparameters and the logger were given quotes to escape semicolons in the argument. On unix, this prevented the argument from being split up by bash. On windows, it seems that the run.cmd/run.exe tools would prevent the extra quotes from being passed to msbuild.exe (desktop msbuild would choke on the quotes if they were passed along). Unlike desktop msbuild, dotnet msbuild is able to parse the quoted strings, so we simply psas the quoted arguments directly to it on windows. We may be able to do the same on unix.
Now that we are using dotnet msbuild directly, this is no longer needed.
When copying native files during the unix test build, we rely on __BuildOS being set. Fixing the import order and always setting __BuildOS fixes this. We should eventually fix the inconsistent use of BuildOS vs __BuildOS.
hoyosjs
approved these changes
Jan 16, 2019
As long as the CI doesn't detect anything weird and we update the Maestro subscription this sounds like a long standing cleanup. Thanks!Was causing unrelated problems. This should be addressed in a separate change.
|
Successful arcade build with latest changes: https://dnceng.visualstudio.com/internal/_build/results?buildId=76197. |
sbomer
changed the title
|
@dotnet-bot help |
|
Welcome to the dotnet/coreclr Perf help The following is a list of valid commands on this PR. To invoke a command, comment the indicated phrase on the PR The following commands are valid for all PRs and repositories. Click to expand
The following jobs are launched by default for each PR against dotnet/coreclr:master. Click to expand
The following optional jobs are available in PRs against dotnet/coreclr:master. Click to expand
Have a nice day! |
|
Welcome to the dotnet/coreclr Repository The following is a list of valid commands on this PR. To invoke a command, comment the indicated phrase on the PR The following commands are valid for all PRs and repositories. Click to expand
The following jobs are launched by default for each PR against dotnet/coreclr:master. Click to expand
The following optional jobs are available in PRs against dotnet/coreclr:master. Click to expand
Have a nice day! |
|
@dotnet-bot test this please |
|
Welcome to the dotnet/coreclr Repository The following is a list of valid commands on this PR. To invoke a command, comment the indicated phrase on the PR The following commands are valid for all PRs and repositories. Click to expand
The following jobs are launched by default for each PR against dotnet/coreclr:master. Click to expand
The following optional jobs are available in PRs against dotnet/coreclr:master. Click to expand
Have a nice day! |
Consistently use __ prefix. Also use consistent casing in build.cmd
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
This moves us one step away from buildtools and towards arcade. This replaces run.sh and run.cmd (and all invocations) with "dotnet.sh msbuild" and "msbuild.cmd". - I'm using these wrapper scripts for now instead of those in eng/common in order to retain the very helpful "Running <command>" output in the build logs. - I'm using msbuild.cmd instead of dotnet.sh to match the current behavior that uses desktop msbuild on windows, instead of dotnet. All of the arguments that used to be implicitly generated by run.exe and config.json are now explicit, resulting in longer (but easier to copy+paste) commands. Some of these arguments are likely unnecessary, but in this change my goal is just to match the run.exe behavior. Later, I would like to go through and clean up parameters that don't need to be passed in every invocation. I might also consider moving more of the common arguments out into variables in a later change. Some of the wrapper scripts now have limited support for parsing "-Argument=Value" style parameters, to support our existing buildpipeline infrastructure, since I thought this was easier to test than changing our buildpipeline definitions. We can remove that parsing logic once we stop using buildpipeline (which has happened at this point). Some subtle parts of the change: * Add msbuild.cmd This simulates the behavior of Tools\msbuild.cmd, which calls desktop msbuild. * Fix BuildOS processing in package build and publish Previously, config.json had its own processing that would set OSName. Instead, we now pass it in explicitly where it's needed (building packages), or not at all (publishing them). * Handle "=" in publish-packages.cmd and other scripts This seems necessary to properly handle the azure access token * Set __BuildOS in PublishPackages Required for cases where the build OS isn't detected during the build such as freebsd. * Use dotnet msbuild in runtest.py This prevents us from having to deal with different quote escape behavior on windows and linux. Previously, arguments like fileloggerparameters and the logger were given quotes to escape semicolons in the argument. On unix, this prevented the argument from being split up by bash. On windows, it seems that the run.cmd/run.exe tools would prevent the extra quotes from being passed to msbuild.exe (desktop msbuild would choke on the quotes if they were passed along). Unlike desktop msbuild, dotnet msbuild is able to parse the quoted strings, so we simply psas the quoted arguments directly to it on windows. We may be able to do the same on unix. * Fix build-test.sh problem with BuildOS When copying native files during the unix test build, we rely on __BuildOS being set. Fixing the import order and always setting __BuildOS fixes this. We should eventually fix the inconsistent use of BuildOS vs __BuildOS. Commit migrated from dotnet/coreclr@1b43bd8
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This moves us one step away from buildtools and towards arcade.
This replaces run.sh and run.cmd (and all invocations) with "dotnet.sh msbuild" and "msbuild.cmd".
All of the arguments that used to be implicitly generated by run.exe and config.json are now explicit, resulting in longer (but easier to copy+paste) commands. Some of these arguments are likely unnecessary, but in this change my goal is just to match the run.exe behavior. Later, I would like to go through and clean up parameters that don't need to be passed in every invocation. I might also consider moving more of the common arguments out into variables in a later change.
Some of the wrapper scripts now have limited support for parsing "-Argument=Value" style parameters, to support our existing buildpipeline infrastructure, since I thought this was easier to test than changing our buildpipeline definitions. We can remove that parsing logic once we stop using buildpipeline (hopefully soon!).
I'm still validating this change with our buildpipeline official builds, so don't merge yet.