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

Move dotnet/format to dotnet/sdk #38857

Merged
merged 3,323 commits into from
Mar 19, 2024
Merged

Move dotnet/format to dotnet/sdk #38857

merged 3,323 commits into from
Mar 19, 2024

Conversation

Cosifne
Copy link
Member

@Cosifne Cosifne commented Feb 17, 2024

This is generated based on 82edc548821f6f36fb4b8542ad47466d65a3a26a in dotnet/format repo.
I pulled the commits from dotnet/format directly, so it will preserve all the history. Let me know if this is not OK.
I get dotnet/format project locally build passed.

All commits before 592b10a are not interested. Mine commits are listed after that. I will put comments in interesting place to help review

sharwell and others added 30 commits October 23, 2023 09:09
Include external access layer for Razor
* Update dependencies from https://github.com/dotnet/roslyn build 20231017.14

Microsoft.CodeAnalysis
 From Version 4.8.0-3.23474.1 -> To Version 4.8.0-3.23517.14

* Update dependencies from https://github.com/dotnet/roslyn build 20231018.7

Microsoft.CodeAnalysis
 From Version 4.8.0-3.23474.1 -> To Version 4.8.0-3.23518.7

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Sam Harwell <[email protected]>
…127-b0af-99a020876755

[release/8.x] Update dependencies from dotnet/arcade
…8-29d299321a94

[main] Update dependencies from dotnet/source-build-externals
…fdc-87ea-0146510626ce

[release/7.x] Update dependencies from dotnet/roslyn
* Update dependencies from https://github.com/dotnet/roslyn build 20231020.1

Microsoft.CodeAnalysis
 From Version 4.8.0-3.23474.1 -> To Version 4.9.0-1.23520.1

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Joey Robichaud <[email protected]>
Co-authored-by: Jan Jones <[email protected]>
* Update dependencies from https://github.com/dotnet/runtime build 20231020.16

Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.Logging
 From Version 8.0.0-rtm.23517.16 -> To Version 8.0.0-rtm.23520.16

* Update dependencies from https://github.com/dotnet/runtime build 20231023.12

Microsoft.Bcl.AsyncInterfaces , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.Logging
 From Version 8.0.0-rtm.23517.16 -> To Version 8.0.0

* Stabilize package version

* Remove package source mapping

* Update Version.Details.xml

* Update azure-pipelines-official.yml

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Jan Jones <[email protected]>
Co-authored-by: Viktor Hofer <[email protected]>
* Update dependencies from https://github.com/dotnet/arcade build 20231018.2

Microsoft.DotNet.Arcade.Sdk
 From Version 9.0.0-beta.23516.5 -> To Version 9.0.0-beta.23518.2

Dependency coherency updates

Microsoft.DotNet.XliffTasks
 From Version 1.0.0-beta.23509.1 -> To Version 1.0.0-beta.23516.1 (parent: Microsoft.DotNet.Arcade.Sdk

* Fix NuGet version

* Update dependencies from https://github.com/dotnet/arcade build 20231018.2

Microsoft.DotNet.Arcade.Sdk
 From Version 9.0.0-beta.23516.5 -> To Version 9.0.0-beta.23518.2

Dependency coherency updates

Microsoft.DotNet.XliffTasks
 From Version 1.0.0-beta.23509.1 -> To Version 1.0.0-beta.23516.1 (parent: Microsoft.DotNet.Arcade.Sdk

* Update dependencies from https://github.com/dotnet/arcade build 20231018.2

Microsoft.DotNet.Arcade.Sdk
 From Version 9.0.0-beta.23516.5 -> To Version 9.0.0-beta.23518.2

Dependency coherency updates

Microsoft.DotNet.XliffTasks
 From Version 1.0.0-beta.23509.1 -> To Version 1.0.0-beta.23516.1 (parent: Microsoft.DotNet.Arcade.Sdk

* Update dependencies from https://github.com/dotnet/arcade build 20231018.2

Microsoft.DotNet.Arcade.Sdk
 From Version 9.0.0-beta.23516.5 -> To Version 9.0.0-beta.23518.2

Dependency coherency updates

Microsoft.DotNet.XliffTasks
 From Version 1.0.0-beta.23509.1 -> To Version 1.0.0-beta.23516.1 (parent: Microsoft.DotNet.Arcade.Sdk

* Update dependencies from https://github.com/dotnet/arcade build 20231018.2

Microsoft.DotNet.Arcade.Sdk
 From Version 9.0.0-beta.23516.5 -> To Version 9.0.0-beta.23518.2

Dependency coherency updates

Microsoft.DotNet.XliffTasks
 From Version 1.0.0-beta.23509.1 -> To Version 1.0.0-beta.23516.1 (parent: Microsoft.DotNet.Arcade.Sdk

* Update dependencies from https://github.com/dotnet/arcade build 20231018.2

Microsoft.DotNet.Arcade.Sdk
 From Version 9.0.0-beta.23516.5 -> To Version 9.0.0-beta.23518.2

Dependency coherency updates

Microsoft.DotNet.XliffTasks
 From Version 1.0.0-beta.23509.1 -> To Version 1.0.0-beta.23516.1 (parent: Microsoft.DotNet.Arcade.Sdk

* Fixup merge

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Jan Jones <[email protected]>
Co-authored-by: Viktor Hofer <[email protected]>
* Update dependencies from https://github.com/dotnet/runtime build 20231017.5

Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.Logging
 From Version 8.0.0-rtm.23513.17 -> To Version 9.0.0-alpha.1.23517.5

* Update dependencies from https://github.com/dotnet/runtime build 20231023.1

Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.Logging
 From Version 8.0.0-rtm.23517.16 -> To Version 9.0.0-alpha.1.23523.1

* Update Version.Details.xml

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Jan Jones <[email protected]>
Co-authored-by: Viktor Hofer <[email protected]>
…024.1 (#1997)

[main] Update dependencies from dotnet/arcade
- Coherency Updates:
  - Microsoft.DotNet.XliffTasks: from 1.0.0-beta.23516.1 to 1.0.0-beta.23523.1 (parent: Microsoft.DotNet.Arcade.Sdk)
…ng/internal/dotnet-runtime

This pull request updates the following dependencies

[marker]: <> (Begin:97b8e377-e3a6-442c-efa9-08dbd545292d)
## From https://dev.azure.com/dnceng/internal/_git/dotnet-runtime
- **Subscription**: 97b8e377-e3a6-442c-efa9-08dbd545292d
- **Build**: 20230926.7
- **Date Produced**: September 26, 2023 11:30:23 PM UTC
- **Commit**: 0b25e38ad32a69cd83ae246104b32449203cc71c
- **Branch**: refs/heads/internal/release/8.0-rc2

[DependencyUpdate]: <> (Begin)

- **Updates**:
  - **Microsoft.Bcl.AsyncInterfaces**: [from 8.0.0 to 8.0.0-rc.2.23476.7][1]
  - **Microsoft.Extensions.FileSystemGlobbing**: [from 8.0.0 to 8.0.0-rc.2.23476.7][1]
  - **Microsoft.Extensions.Logging**: [from 8.0.0 to 8.0.0-rc.2.23476.7][1]
  - **Microsoft.SourceBuild.Intermediate.runtime.linux-x64**: [from 8.0.0-rtm.23523.12 to 8.0.0-rc.2.23476.7][1]

[1]: https://dev.azure.com/dnceng/internal/_git/dotnet-runtime/branches?baseVersion=GC488a8a3521&targetVersion=GC0b25e38ad3&_a=files

[DependencyUpdate]: <> (End)

[marker]: <> (End:97b8e377-e3a6-442c-efa9-08dbd545292d)
…ng/internal/dotnet-runtime

This pull request updates the following dependencies

[marker]: <> (Begin:97b8e377-e3a6-442c-efa9-08dbd545292d)
## From https://dev.azure.com/dnceng/internal/_git/dotnet-runtime
- **Subscription**: 97b8e377-e3a6-442c-efa9-08dbd545292d
- **Build**: 20231025.7
- **Date Produced**: October 25, 2023 9:10:32 PM UTC
- **Commit**: 17ea9ab3f662320d4ac62d3a2b783b5054ad80bf
- **Branch**: refs/heads/internal/release/8.0

[DependencyUpdate]: <> (Begin)

- **Updates**:
  - **Microsoft.Bcl.AsyncInterfaces**: [from 8.0.0-rc.2.23476.7 to 8.0.0][1]
  - **Microsoft.Extensions.FileSystemGlobbing**: [from 8.0.0-rc.2.23476.7 to 8.0.0][1]
  - **Microsoft.Extensions.Logging**: [from 8.0.0-rc.2.23476.7 to 8.0.0][1]
  - **Microsoft.SourceBuild.Intermediate.runtime.linux-x64**: [from 8.0.0-rc.2.23476.7 to 8.0.0-rtm.23525.7][1]

[1]: https://dev.azure.com/dnceng/internal/_git/dotnet-runtime/branches?baseVersion=GC0b25e38ad3&targetVersion=GC17ea9ab3f6&_a=files

[DependencyUpdate]: <> (End)

[marker]: <> (End:97b8e377-e3a6-442c-efa9-08dbd545292d)
Copy link
Member

@MiYanni MiYanni left a comment

Choose a reason for hiding this comment

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

Other things to mention:

  • The repo using global usings, so likely 70% or more of the using statements in your source files are unnecessary.
  • The tests should be converted to Helix tests at some point, but it is not necessary in this PR to do that.

documentation/format/docs/test-plan.md Outdated Show resolved Hide resolved
<Dependency Name="dotnet-format" Version="9.0.512801">
<Uri>https://github.com/dotnet/format</Uri>
<Sha>de0824fccb49f4c23a9a55c6d35b1f1fb23bcfa5</Sha>
</Dependency>
<!-- Intermediate is necessary for source build. -->
<Dependency Name="Microsoft.SourceBuild.Intermediate.format" Version="9.0.512801">
<Uri>https://github.com/dotnet/format</Uri>
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK, this should be removed. Not sure if adjustments need to be made on the sourcebuild side.


if ($stage -eq "format-workspace") {
Write-Output "$(Get-Date) - $solutionFile - Formatting Workspace"
$output = & $parentDotNetPath "$currentLocation/artifacts/bin/dotnet-format/Release/net9.0/dotnet-format.dll" $solution --no-restore -v diag --verify-no-changes | Out-String
Copy link
Member

Choose a reason for hiding this comment

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

Seems suspicious to have net9.0 folder hardcoded in the path.


if ($stage -eq "format-folder") {
Write-Output "$(Get-Date) - $folderName - Formatting Folder"
$output = & $parentDotNetPath "$currentLocation/artifacts/bin/dotnet-format/Release/net9.0/dotnet-format.dll" whitespace $repoPath --folder -v diag --verify-no-changes | Out-String
Copy link
Member

Choose a reason for hiding this comment

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

Same here with net9.0 folder hardcoded in the path.

Copy link
Member Author

Choose a reason for hiding this comment

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

If TFM get updates, will it be OK to change the value here when the build failed?

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 guess TFM is now maintained by arcade using NetCurrent, not sure if there a good way to get that value in normal script

Copy link
Member

Choose a reason for hiding this comment

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

What I was indicating is that you can just find the dll. You know the name of it, and you know the directory structure up until the point where it could change. So, just before this line add:

$dllPath = Get-ChildItem -Path "$currentLocation/artifacts/bin/dotnet-format/Release/" -Include dotnet-format.dll -Recurse

And then this line would be:

$output = & $parentDotNetPath "$($dllPath.FullName)" whitespace $repoPath --folder -v diag --verify-no-changes | Out-String

src/Assets/dotnet-format/Directory.Build.props Outdated Show resolved Hide resolved
test/Directory.Build.targets Outdated Show resolved Hide resolved
@MichaelSimons
Copy link
Member

MichaelSimons commented Mar 7, 2024

I think it would be valuable to validated this change in the context of the VMR to unsure this doesn't break. A break would block the dependency from of the SDK into installer which is something to try to avoid. To validate:

  1. Pull the VMR locally

  2. Create a working branch

  3. Sync these change into the VMR using the https://github.com/dotnet/dotnet/blob/main/src/installer/eng/vmr-sync.sh script.

    <path to your local VMR>/vmr-sync.sh \
       --repository sdk:3e70a96b84932215a4e77bc2ec9c7b667e2a1679 \
       --sdk sdk:https://github.com/dotnet/sdk         \
       --tmp "$HOME/repos/tmp"
       --vmr "<path to your local VMR>
    

    cc @premun on usage of vmr-sync

    After syncing, you will want to remove this reference - https://github.com/dotnet/dotnet/blob/main/repo-projects/sdk.proj#L21

  4. Create a PR and let CI validate the change.

@Cosifne
Copy link
Member Author

Cosifne commented Mar 13, 2024

VMR test PR
dotnet/dotnet#79

@Cosifne
Copy link
Member Author

Cosifne commented Mar 15, 2024

VMR build passed

Copy link
Member

@MiYanni MiYanni left a comment

Choose a reason for hiding this comment

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

Looks good!


if ($stage -eq "format-folder") {
Write-Output "$(Get-Date) - $folderName - Formatting Folder"
$output = & $parentDotNetPath "$currentLocation/artifacts/bin/dotnet-format/Release/net9.0/dotnet-format.dll" whitespace $repoPath --folder -v diag --verify-no-changes | Out-String
Copy link
Member

Choose a reason for hiding this comment

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

What I was indicating is that you can just find the dll. You know the name of it, and you know the directory structure up until the point where it could change. So, just before this line add:

$dllPath = Get-ChildItem -Path "$currentLocation/artifacts/bin/dotnet-format/Release/" -Include dotnet-format.dll -Recurse

And then this line would be:

$output = & $parentDotNetPath "$($dllPath.FullName)" whitespace $repoPath --folder -v diag --verify-no-changes | Out-String

@Cosifne
Copy link
Member Author

Cosifne commented Mar 15, 2024

@MiYanni
Does sdk team prefer to keep the commit history from dotnet/format? Or should I squash all commits into one commit for a clean history?

@MiYanni
Copy link
Member

MiYanni commented Mar 15, 2024

@Cosifne You can keep the history. That's fine. This PR cannot be merged, though, until after the preview snap, which I think is... Monday? You can confirm that with @marcpopMSFT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.