Skip to content

Conversation

@powercode
Copy link
Collaborator

@powercode powercode commented Oct 24, 2017

#3400

Adding TargetFramework property to csprojs. This needs to be in each project to work around a visual studio issue where it doesn't react to TargetFramework set in an imported file.
Updating sln to reflect the change of project types to .net core
removing obsolete resources that makes the build fail.

At least I think the resources are obsolete :)
Hope reviewers can verify.

@iSazonov
Copy link
Collaborator

I think we should add TargetFramework property to PowerShell.Common.props not every csproj.

@powercode
Copy link
Collaborator Author

The whole point of having it in every project is to work around the issue that visual studio issue doesn't understand TargetFramework from imported files.

@powercode
Copy link
Collaborator Author

The property already is in PowerShell.Common.props.

@iSazonov
Copy link
Collaborator

I wonder how and why the trick works.

@powercode
Copy link
Collaborator Author

My guess is that they explicitly look for the property in the file, but using the full logic for evalutating it's value.

@powercode
Copy link
Collaborator Author

By doing this, we can at least keep the value consistent, and updatable in one place, while still being able to use Visual Studio.

I agree it isn't ideal, and it should be removed if VS fixes the issue.
I haven't checked where this can be reported. @PowerShellTeam anyone got any good connections in devdiv?

@iSazonov
Copy link
Collaborator

It would be good to have a comment from VS team that is best way in the case.

@powercode
Copy link
Collaborator Author

And maybe a comment in the csproj files why that strange construct is used.

@Bhaal22
Copy link
Contributor

Bhaal22 commented Nov 28, 2017

@this issue looks to be related to the issue experienced here:

dotnet/project-system#1358

@bergmeister
Copy link
Contributor

bergmeister commented Dec 2, 2017

@powercode
I have tried your fork on my local machine. I ran Start-PSBootStrap, installed the .Net Core SDK 2.0.0 (since this is the state of your fork), ran a dotnet restore on the solution and had to pre-build it using Start-PSBuild. Then trying to build in VS still yields some errors:
image
Also I have seen that all projects are practically empty in Solution Explorer because in all the projects source code is marked as <Compile Remove=*.cs>. We need to be able to see the source code files in VS in order for it to be useful. How does the whole build system work if the source code files are removed from compilation, does the TypeCatalogGen do all that work?

@powercode
Copy link
Collaborator Author

powercode commented Dec 15, 2017

@bergmeister That is wierd. That reference is removed in c8e4192

Are you sure you are running on my branch? Without these changes, all source files are hidden in Visual Studio, but with the TargetFrameworkVersion, it seems to work as expected.

@bergmeister
Copy link
Contributor

@powercode Sorry, I was caught out by cloning your fork but being on the wrong branch. I can see the files now in solution explorer but building only seems to work if the repo is pre-build via command line via Start-PSBuild (just running Start-ResGen is not sufficient). I am using using VS 15.5.2 on Win10.

@TravisEz13 TravisEz13 changed the title Make powershell build in Visual Studio Build: Make powershell build in Visual Studio Dec 15, 2017
@bergmeister
Copy link
Contributor

@powercode : can you please downstream changes from master to your fork since GitHub does not allow to have multiple forks so that people can start using it already. What timeline do we expect until this is merged into master @TravisEz13 ?
Downstreaming can be done as follows:

cd $PowerShellCheckout
git checkout vs-csproj
git remote add upstream https://github.com/PowerShell/PowerShell.git
git pull upstream master
git push

@TravisEz13
Copy link
Member

@bergmeister , @daxian-dbw is assigned to this PR.
I've restarted Travis-CI linux due to markdown test failures due to out of date code. hopefully github will remerge on it's own.

@SteveL-MSFT SteveL-MSFT added this to the 6.1.0-MQ milestone Jan 18, 2018
@powercode
Copy link
Collaborator Author

Rebased on master.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please clarify why we need only "Release"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That may have been unintentional. Seems to me, that our platform/config system is not really well designed, but it is not that important here. To begin with, this is just so we can start iterating faster on the code from Visual Studio. It is not affecting the build of the product.

@iSazonov
Copy link
Collaborator

@markekraus Do you use Visual Studio? Could you review the PR?

@markekraus
Copy link
Contributor

@iSazonov I have Visual Studio, but I'm not familiar with it (I use VS Code). You are better off asking for a review @rkeithhill and/or @bergmeister. I believe they are both familiar with Visual Studio.

@bergmeister
Copy link
Contributor

I can take a look again since I have to use VS nearly the whole day but my review from a user's perspective is already here
But it definitely makes sense if others chip in as well.

@iSazonov
Copy link
Collaborator

@daxian-dbw Could you please continue with the PR?

@daxian-dbw
Copy link
Member

We need to actually use the changes for a few days to get a feel. I suggest all active maintainers to try out the changes. /cc @PowerShell/powershell-maintainers
Also, for anyone who want/like this PR, please also try out the changes and help bring up issues. The more eyes the merrier :)

@iSazonov
Copy link
Collaborator

iSazonov commented Jan 26, 2018

  1. Step

git clean -fdX
run Build in VS

========== Rebuild All: 0 succeeded, 13 failed, 0 skipped ==========

I found related dotnet/project-system#2129

  1. Step

dotenet restore
run Build in VS

We see two errors "Invalid Resx file" with xsd.
src\System.Management.Automation\resources\CmdletizationCoreResources.resx
src\Microsoft.PowerShell.Commands.Management\resources\CmdletizationResources.resx

3.Step

After fix this Resx files
run Build in VS

visual studio cannot start debugging because the debug target is missing exe

We haven't compiled resources.

  1. Step

run our RexGen
run Build in VS

We haven't "InitializeTypeCatalog"

  1. Step

run our TypeGen
run Build in VS

Now we build well.

6 Step

run from VS
or
run debugging in VS

Get error: visual studio cannot start debugging because the target '\pwsh.exe' is missing
Really there is no thefile - only pwsh.dll

VS2017trace.txt

@powercode
Copy link
Collaborator Author

Until we move resource generation into a msbuild task, we will have to complete a cmdline build first.

I set up a publishing rule to publish a build before debugging, and manually configured the debug target.
Debugging is generally not something you share, since you often start with different arguments.

@iSazonov
Copy link
Collaborator

I already push PR #3870 for TypeGen but it don't approved a long time. I already patched our temp utility for ResGen in PR #3918 and ready to push PR to run it by MSBuild.

@lzybkr lzybkr removed their request for review March 14, 2018 16:26
@daxian-dbw daxian-dbw assigned TravisEz13 and unassigned daxian-dbw Apr 3, 2018
@TravisEz13
Copy link
Member

I created a new PR based on this one. #6546 I've verified it works and the fix is simpler. I based it on your fix. I think we should probably close this one. Thanks, Travis . Thanks for your help.

@TravisEz13 TravisEz13 closed this Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants