Skip to content

Add workflow to update solution files with SlnGen and update them initially#44703

Merged
ViktorHofer merged 2 commits intodotnet:masterfrom
ViktorHofer:UpdateSlns
Nov 17, 2020
Merged

Add workflow to update solution files with SlnGen and update them initially#44703
ViktorHofer merged 2 commits intodotnet:masterfrom
ViktorHofer:UpdateSlns

Conversation

@ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Nov 15, 2020 •
edited
Loading

Fixes #43109
Fixes #42121

SlnGen is used to update the solution files. Adding:

  • Global tool manifest entry to slngen
  • Script to update the solution files

Updating all the solution files checked in. The size increases are expected as a) SlnGen adds dedicated nodes for the solution folders and b) missing dependencies are now part of the solution file.

@ViktorHofer ViktorHofer requested a review from Anipik November 15, 2020 15:50
@ViktorHofer ViktorHofer self-assigned this Nov 15, 2020
@ghost
Copy link

ghost commented Nov 15, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Details

Issue Details
Description:

SlnGen is used to update the solution files. Adding:

  • Global tool manifest entry to slngen
  • Script to update the solution files

Updating all the solution files checked in. The size increases are expected as a) SlnGen adds dedicated nodes for the solution folders and b) missing dependencies are now part of the solution file.
Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Infrastructure-libraries
Milestone: -

@ViktorHofer ViktorHofer changed the title Add workflow to update solution files with SlnGen and the solutions initially Add workflow to update solution files with SlnGen and update them initially Nov 15, 2020
@ViktorHofer ViktorHofer mentioned this pull request Nov 15, 2020
Closed
@jkotas
Copy link
Member

jkotas commented Nov 15, 2020

Is it intentional that the .sln files are added as binary files?

@ViktorHofer ViktorHofer added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 15, 2020
@ViktorHofer
Copy link
Member Author

ViktorHofer commented Nov 15, 2020 •
edited
Loading

Is it intentional that the .sln files are added as binary files?

It's not. Investigating...

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Nov 15, 2020 •
edited
Loading

Hmm no idea:

C:\git\runtime3\src\libraries\System.Text.RegularExpressions>git check-attr -a System.Text.RegularExpressions.sln
System.Text.RegularExpressions.sln: text: set
System.Text.RegularExpressions.sln: eol: crlf

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Nov 15, 2020 •
edited
Loading

OK reason is, the file encoding is UTF-16LE which GH detects as binary. PR in slngen: microsoft/slngen#189

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Nov 15, 2020

Fixed, removing the no-merge label.

@ViktorHofer ViktorHofer removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 15, 2020
Anipik
Anipik approved these changes Nov 16, 2020
View reviewed changes
Copy link
Contributor

@Anipik Anipik left a comment

Choose a reason for hiding this comment

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

LGTM

@ViktorHofer ViktorHofer mentioned this pull request Nov 16, 2020
Closed
@ViktorHofer ViktorHofer requested a review from vargaz as a code owner November 16, 2020 11:14
@ViktorHofer
Copy link
Member Author

ViktorHofer commented Nov 16, 2020

/azp run runtime

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Nov 16, 2020

/azp run runtime-live-build

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 16, 2020

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

azure-pipelines bot commented Nov 16, 2020

Azure Pipelines successfully started running 1 pipeline(s).

akoeplinger
akoeplinger reviewed Nov 17, 2020
View reviewed changes
src/libraries/UpdateLibrariesSlns.ps1
$ProjContent | Out-File -FilePath $ProjFilePath
dotnet slngen "$ProjFilePath -p SlnGenMainProject=$($file.BaseName) --launch false --nologo"
Remove-Item $ProjFilePath
}
Copy link
Member

@akoeplinger akoeplinger Nov 17, 2020

Choose a reason for hiding this comment

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

could you rewrite this in msbuild so we can run it on macOS/Linux too?

Copy link
Member Author

@ViktorHofer ViktorHofer Nov 17, 2020

Choose a reason for hiding this comment

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

Powershell runs on macOS & Linux. We have couple other ps1 scripts which we only trigger on demand like this one.

Copy link
Member

@akoeplinger akoeplinger Nov 20, 2020

Choose a reason for hiding this comment

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

Ah right, I keep forgetting about cross-platform powershell :D

@ViktorHofer ViktorHofer merged commit ed29578 into dotnet:master Nov 17, 2020
@ViktorHofer ViktorHofer deleted the UpdateSlns branch November 17, 2020 17:23
@ghost ghost locked as resolved and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akoeplinger akoeplinger akoeplinger left review comments

@EgorBo EgorBo Awaiting requested review from EgorBo

@marek-safar marek-safar Awaiting requested review from marek-safar

@steveisok steveisok Awaiting requested review from steveisok

@vargaz vargaz Awaiting requested review from vargaz

+1 more reviewer

@Anipik Anipik Anipik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ViktorHofer ViktorHofer

Labels

area-Infrastructure-libraries

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Solution Files in libraries don't list all required dependencies Issues while building trying to use the Test Explorer in certain solutions

4 participants

@ViktorHofer @jkotas @akoeplinger @Anipik