8000 Move Start-TypeGen logic from Build.psm1 to SDK csproj file by iSazonov · Pull Request #3870 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

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

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

Closed
wants to merge 1 commit into from

Conversation

iSazonov
Copy link
Collaborator
@iSazonov iSazonov commented May 26, 2017

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)

@iSazonov
Copy link
Collaborator Author
iSazonov commented Jun 1, 2017

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

@daxian-dbw
Copy link
Member

@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 @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.

@iSazonov
Copy link
Collaborator Author

@TravisEz13 @daxian-dbw Could you please continue the code review?

@TravisEz13 TravisEz13 added the Area-Maintainers-Build specific to affecting the build label Jun 14, 2017
@TravisEz13
Copy link
Member

@daxian-dbw Are you ok with this change now?

@daxian-dbw
Copy link
Member

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

@daxian-dbw daxian-dbw assigned daxian-dbw and unassigned TravisEz13 Jun 23, 2017
@iSazonov
Copy link
Collaborator Author

@daxian-dbw Could you please review?

@daxian-dbw daxian-dbw requested a review from TravisEz13 August 18, 2017 23:16
@daxian-dbw
Copy link
Member

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

@iSazonov
Copy link
Collaborator Author
iSazonov commented Aug 19, 2017

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.).

@iSazonov iSazonov force-pushed the msbuild-typecataloggen branch from 09fd8a9 to 6717fcc Compare August 21, 2017 09:37
@daxian-dbw
Copy link
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.)

@TravisEz13
Copy link
Member

@daxian-dbw Do you want to review?

@stale
Copy link
stale bot commented Apr 13, 2018

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 stale bot added the Stale label Apr 13, 2018
@stale stale bot removed the Stale label Apr 20, 2018
@stale
Copy link
stale bot commented May 20, 2018

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 stale bot added the Stale label May 20, 2018
@iSazonov
Copy link
Collaborator Author

Should I fix or close the PR?

/cc @daxian-dbw

@stale stale bot removed the Stale label May 21, 2018
@daxian-dbw
Copy link
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.

@iSazonov iSazonov closed this May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Maintainers-Build specific to affecting the build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0