Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Closed
iSazonov wants to merge 1 commit into
PowerShell:masterPowerShell/PowerShell:masterfrom
iSazonov:msbuild-typecataloggeniSazonov/PowerShell:msbuild-typecataloggenCopy head branch name to clipboard
Closed

Move Start-TypeGen logic from Build.psm1 to SDK csproj file#3870
iSazonov wants to merge 1 commit into
PowerShell:masterPowerShell/PowerShell:masterfrom
iSazonov:msbuild-typecataloggeniSazonov/PowerShell:msbuild-typecataloggenCopy head branch name to clipboard

Conversation

@iSazonov

@iSazonov iSazonov commented May 26, 2017

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)

@iSazonov

iSazonov commented Jun 1, 2017

Copy link
Copy Markdown
Collaborator Author

@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
Copy Markdown
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
Copy Markdown
Collaborator Author

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

@TravisEz13 TravisEz13 added the WG-Maintainers-Build specific to affecting the build label Jun 14, 2017
@TravisEz13

Copy link
Copy Markdown
Member

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

@daxian-dbw

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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

@iSazonov

iSazonov commented Aug 19, 2017

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

@iSazonov iSazonov force-pushed the msbuild-typecataloggen branch from 09fd8a9 to 6717fcc Compare August 21, 2017 09:37
@daxian-dbw

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

@TravisEz13

Copy link
Copy Markdown
Member

@daxian-dbw Do you want to review?

@stale

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 stale Bot added the Stale label Apr 13, 2018
@stale stale Bot removed the Stale label Apr 20, 2018
@stale

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 stale Bot added the Stale label May 20, 2018
@iSazonov

Copy link
Copy Markdown
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
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.

@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

WG-Maintainers-Build specific to affecting the build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.