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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Nov 17, 2020
Merged

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

merged 2 commits into from
Nov 17, 2020

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.

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

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

/azp run runtime

@ViktorHofer
Copy link
Member Author

/azp run runtime-live-build

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

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

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

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

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

@Anipik Anipik Anipik approved these changes

@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

Assignees

@ViktorHofer ViktorHofer

Labels
area-Infrastructure-libraries
Projects
None yet
Milestone
No milestone
4 participants
@ViktorHofer @jkotas @akoeplinger @Anipik