Skip to content

Eliminate build warnings - possible update some pkg refs #558

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
rkeithhill opened this issue Oct 22, 2017 · 20 comments
Closed

Eliminate build warnings - possible update some pkg refs #558

rkeithhill opened this issue Oct 22, 2017 · 20 comments
Labels

Comments

@rkeithhill
Copy link
Contributor

When I build, I see a number of NU1603 warnings like this:

/home/hillr/github/rkeithhill/PowerShellEditorServices/src/PowerShellEditorServices.Host/PowerShellEditorServices.Host.csproj : warning NU1603: Microsoft.NETCore.Portable.Compatibility 1.0.3-beta-24514-00 depends on Microsoft.NETCore.Runtime.CoreCLR (>= 1.0.2-beta-24512-03) but Microsoft.NETCore.Runtime.CoreCLR 1.0.2-beta-24512-03 was not found. An approximate best match of Microsoft.NETCore.Runtime.CoreCLR 1.0.2-rc2-23818 was resolved.

I also note that the project files are referencing a 10 month old version of the MS.PS.Sdk -alpha13.

Should these be updated to a more recent version?

@daviwil
Copy link
Contributor

daviwil commented Oct 23, 2017

I think I am building against the older alpha package for the PS SDK because I need access to PS 5.1 APIs that aren't available in the PS 5.0 SDK package on NuGet. I tried changing to a more up to date version but ran into trouble. You can certainly give it a shot if you like!

@rkeithhill
Copy link
Contributor Author

rkeithhill commented Oct 24, 2017

I think we'd need to wait until there is a PowerShellStandard.Library v5.1. At that time, do you think we can get away with supporting v3 and v5.1 in addition to PS Core 6 and dropping support for v4 and v5.0?

@SteveL-MSFT
Copy link
Member

PS Std 5.1 Library should be published no later than end of this month. Is there data to suggest v3 or v4 users? 5.1 is a mandatory update over 5.0 to get support so dropping 5.0 makes sense.

@SeeminglyScience
Copy link
Collaborator

Maybe this discussion should be a separate issue, but I think there's a strong case for dropping 3/4 support.

There's two bugs I can think of off the top of my head that I've ran into testing PSv3 that haven't been reported yet.

  1. The script we use to find and replace aliases uses the Where magic method that was added in 4. Not a big deal, I can see it not being reported.

  2. We use nested pipelines to run commands on PSv3 when the debugger is stopped. Unlike Debugger.ProcessCommand, nested pipelines still hit breakpoints. Last I checked we don't correctly handle hitting a breakpoint while already in a stop event, resulting in a crash. When I was testing PSv3 even typing on a line with a breakpoint would crash when TabExpansion2 was called.

I can't test at the moment, but if the latter still repros I have a hard time believing anyone is still using PSv3 on a machine they're writing scripts with.

Support for older versions is also the only thing holding back the debugger history fix, and when PSReadLine is ready it will only work on PSv5.1+ because that's what PSReadLine 2.0 requires.

I like the idea of supporting 3 and 4, but I think it's starting to make a lot less sense. I'd be interested to see what happens if we added a warning message saying support for 3/4 is deprecated and will be removed in the future.

@rkeithhill
Copy link
Contributor Author

I wouldn't mind dropping support for v3/v4. That is one thing that has been holding us back from providing symbol support for class & enum. We're using an older, v3 compatible version of the AST (IIRC) that doesn't know about those.

OTOH I really wish we had some VSCode extension download metrics on which OS's have been used to download. If the % of Win7 downloads is still high we might not be able to drop support for those versions just yet. Also, such a change would necessitate bumping the major version IMO.

I suppose we could branch and maintain two versions: the current version with support back to v3 - would get bug fixes only on a v1.x maintenance branch and switch develop over to 2.x and rip out support for v3/v4. But that's a fair amount of extra work to do and maintain. Hmm...

@daviwil
Copy link
Contributor

daviwil commented Jan 11, 2018

I agree, we'd need to do a major version bump if we dropped support for earlier PS versions. It'd certainly make the code easier to maintain. Perhaps it'd be possible to support PSES 1.x side by side with PSES 2.x while not continuing new investment on 1.x.

We could definitely start working on a PSES 2.0 branch and allow it to be turned on with a feature flag. It'd be a good opportunity to finish the cleanup and API redesign work I started in mid-2017.

@SteveL-MSFT, what is your opinion on potentialy dropping support for v3 and v4 if it seems that the majority of users are on 5+?

@rkeithhill
Copy link
Contributor Author

BTW given the blow-back on the removal of the sc alias from PS Core 6, we should make sure we communicate such a decision to the community. :-)

@daviwil
Copy link
Contributor

daviwil commented Jan 11, 2018

Agreed. I think Tyler is about to do a poll on Twitter to ask about it, then we can write something up to get the word out ahead of any actual work on removing them.

@daviwil
Copy link
Contributor

daviwil commented Jan 11, 2018

One nice way to do this could be to create a discussion issue on vscode-powershell and add a popup in the next update of the extension that shows up for anyone using the extension with PS 3/4/5.0 with a link to the issue so that they can comment and identify themselves.

@TylerLeonhardt
Copy link
Member

Here's the poll:
https://twitter.com/TylerLeonhardt/status/951510118704627712

right now it's at about 30% of folks still using 3/4 so we might not be able to deprecate. That said, it's still early.

@rkeithhill
Copy link
Contributor Author

Bummer, Tweetium does not render the poll UI:

image

@TylerLeonhardt
Copy link
Member

feel free to RT so we can reach others too - my PowerShell network is not nearly as large as any of you

@SeeminglyScience
Copy link
Collaborator

Created a reddit thread linking to the tweet for some extra visibility.

@SeeminglyScience
Copy link
Collaborator

I think a lot of the responses we get will be more about the version of PowerShell they write for. Right now there isn't really anything great for validating a script against a target PowerShell version. I've been on and off writing a tool that scrapes changes from the assemblies of different versions to create the data needed to write those tools.

If we decide to move forward with dropping support for PSv3/4 I'll prioritize getting that data ready.

@daviwil
Copy link
Contributor

daviwil commented Jan 11, 2018

Yeah, I think it's more important to have a good editing experience to make code that works on older versions, not necessarily authored while using an older version. We've never really had the former, so it'll be great to make any progress there!

@rkeithhill
Copy link
Contributor Author

@SeeminglyScience I've been asking for such functionality from PSSA for a while now. I believe that is probably the right place for it. I should be able to specify to PSSA the versions of PS my script needs to run on and it should be able to warn me on missing operators, commands, parameters, keywords, etc.

PSSA could use some community love. :-)

@TylerLeonhardt
Copy link
Member

So we're at 11% for 3, 9% for 4.

I'd like to hear what @SteveL-MSFT has to say about this.

@SteveL-MSFT
Copy link
Member

My opinion is that v3 and v4 users can still use ISE. If we can be more efficient and the majority of users are v5.1+, then it seems we should drop v3/4 support.

@daviwil
Copy link
Contributor

daviwil commented Jan 12, 2018

I'd definitely like to find a way to do it "gracefully" that enables things to work for older platforms while unencumbering us to do new work. Hiding the v3/v4 code behind some well factored interfaces, living its own assembly could do the trick.

@SydneyhSmith
Copy link
Collaborator

Closing as this is being tracked in #1310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants