FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Move Start-TypeGen logic from Build.psm1 to SDK csproj file by iSazonov · Pull Request #3870 · PowerShell/PowerShell · GitHub

Move Start-TypeGen logic from Build.psm1 to SDK csproj file - #3870

Closed
Ilya (iSazonov) wants to merge 1 commit into
PowerShell:masterfrom
iSazonov:msbuild-typecataloggen
Closed

Move Start-TypeGen logic from Build.psm1 to SDK csproj file#3870
Ilya (iSazonov) wants to merge 1 commit into
PowerShell:masterfrom
iSazonov:msbuild-typecataloggen

Conversation

Ilya (iSazonov) commented May 26, 2017
edited
Loading

Copy link
Copy Markdown
Collaborator

Related #3400
(Also it is one step to unblock #3690)

Now Microsoft.PowerShell.SDK.csproj:

  1. Generate Reference Assembly List ("RefAssemblyList.inc" file) in project local sub directory "gen" (Target Name="TypeCatalogGen"):
  • Check if Reference Assembly List is updated and only then overwrite RefAssemblyList.inc
  • If RefAssemblyList.inc is updated then generate new CorePsTypeCatalog.cs
  1. Support Clean target to remove RefAssemblyList.inc (for rebuild)

Copy link
Copy Markdown
Collaborator Author

Dongbo Wang (@daxian-dbw) I think I'm a little late with the PR, but it could still be useful for a while.

Copy link
Copy Markdown
Member

Ilya (@iSazonov) We still need to have TypeCatalogGen in our build. With the type catalog, you can use a .NET type without caring whether the assembly that contains it is already loaded because powershell is able to look it up in the type catalog and load the assembly automatically. So it would be a breaking change if we remove the type catalog. And TypeCatalogGen will be useful for new features too, for example, the extension methods support Staffan Gustafsson (@powercode) is working on -- we will need to do some analysis of all reference assemblies at build time to build a cache of all .NET extension methods, which will improve the runtime performance. So this PR is not late 😄
BTW, I'm busy on other tasks recently and thus is slow on code review, sorry for that.

Copy link
Copy Markdown
Collaborator Author

Travis Plunk (@TravisEz13) Dongbo Wang (@daxian-dbw) Could you please continue the code review?

Travis Plunk (TravisEz13) added the WG-Maintainers-Build specific to affecting the build label Jun 14, 2017

Copy link
Copy Markdown
Member

Dongbo Wang (@daxian-dbw) Are you ok with this change now?

Copy link
Copy Markdown
Member

Travis Plunk (@TravisEz13) I haven't got the time to carefully review this yet. Will do it soon.

Copy link
Copy Markdown
Collaborator Author

Dongbo Wang (@daxian-dbw) Could you please review?

Copy link
Copy Markdown
Member

Will the TypeCatalogGen target execute when we running dotnet restore or dotnet build on Microsoft.PowerShell.SDK.csproj?

Ilya (iSazonov) commented Aug 19, 2017
edited
Loading

Copy link
Copy Markdown
Collaborator Author

We call dotnet msbuild .\Microsoft.PowerShell.SDK.csproj /t:"Clean;TypeCatalogGen" "/property:DesignTimeBuild=true" in Build.psm1 in the same point as before - so the behavior must be the same.

Ah, sorry - your question of a direct call - it needs to be checked explicitly because dependencies is deeply.

Update: I preserve current logic - currently we call Start-PSBuild -TypeGen (and dotnet msbuild .\Microsoft.PowerShell.SDK.csproj /t:_GetDependencies "/property:DesignTimeBuild=true;_DependencyFile=$ps_inc_file" /nologo) to update the type catalog and also the type catalog file CorePsTypeCatalog.cs will be generated if it absent (or changed, or we have updates in type references.).

Copy link
Copy Markdown
Member

Great. My concern was that the type catalog (both .inc and .cs) would be generated when just running Start-PSBuild for a build. I will continue the review.
(BTW, I took Monday off, so please don't be concerned if I didn't reply promptly.)

Copy link
Copy Markdown
Member

Dongbo Wang (@daxian-dbw) Do you want to review?

stale Bot commented Apr 13, 2018

Copy link
Copy Markdown

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

stale Bot added the Stale label Apr 13, 2018
stale Bot removed the Stale label Apr 20, 2018

stale Bot commented May 20, 2018

Copy link
Copy Markdown

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

stale Bot added the Stale label May 20, 2018

Copy link
Copy Markdown
Collaborator Author

Should I fix or close the PR?

/cc Dongbo Wang (@daxian-dbw)

stale Bot removed the Stale label May 21, 2018

Copy link
Copy Markdown
Member

Sorry that I completely lost the context here. I won't have much time for code review recently. I'm fine we close this PR for now and we can revive it when needed.

This file contains hidden or 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WG-Maintainers-Build specific to affecting the build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL