-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Build: Make powershell build in Visual Studio #5209
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
Conversation
|
I think we should add |
|
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. |
|
The property already is in |
|
I wonder how and why the trick works. |
|
My guess is that they explicitly look for the property in the file, but using the full logic for evalutating it's value. |
|
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. |
|
It would be good to have a comment from VS team that is best way in the case. |
|
And maybe a comment in the csproj files why that strange construct is used. |
|
@this issue looks to be related to the issue experienced here: |
|
@powercode |
|
@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. |
|
@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 |
|
@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 ? cd $PowerShellCheckout
git checkout vs-csproj
git remote add upstream https://github.com/PowerShell/PowerShell.git
git pull upstream master
git push |
|
@bergmeister , @daxian-dbw is assigned to this PR. |
|
Rebased on master. |
PowerShell-Win.sln
Outdated
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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.
|
@markekraus Do you use Visual Studio? Could you review the PR? |
|
@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. |
|
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 |
|
@daxian-dbw Could you please continue with the PR? |
|
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 |
git clean -fdX ========== Rebuild All: 0 succeeded, 13 failed, 0 skipped ========== I found related dotnet/project-system#2129
dotenet restore We see two errors "Invalid Resx file" with xsd. 3.Step After fix this Resx files visual studio cannot start debugging because the debug target is missing exe We haven't compiled resources.
run our RexGen We haven't "InitializeTypeCatalog"
run our TypeGen Now we build well. 6 Step run from VS Get error: visual studio cannot start debugging because the target '\pwsh.exe' is missing |
|
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. |
This reverts commit 9cbf601.
|
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. |

#3400
Adding
TargetFrameworkproperty 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.