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

Modify InvokeDotnetEf in EntityFramework.psm1 to use .Path instead of .Source to discover dotnet location #5327

Closed
pranavkm opened this Issue May 10, 2016 · 11 comments

Comments

Projects
None yet
5 participants
@pranavkm
Member

pranavkm commented May 10, 2016

According to https://msdn.microsoft.com/en-us/library/system.management.automation.applicationinfo_members(v=vs.85).aspx, the .Source property on ApplicationInfo returned by Get-Command was added in Powershell 5.0. .Path returns the same value and works in powershell 3.0.

Line of code to change: https://github.com/aspnet/EntityFramework/blob/dev/src/Microsoft.EntityFrameworkCore.Tools/tools/EntityFramework.psm1#L643

@pranavkm

This comment has been minimized.

Show comment
Hide comment
@pranavkm

pranavkm May 10, 2016

Member

To perform EF migrations in the package manager console until this is fixed, install powershell 5.0.

Member

pranavkm commented May 10, 2016

To perform EF migrations in the package manager console until this is fixed, install powershell 5.0.

@divega

This comment has been minimized.

Show comment
Hide comment
@divega

divega May 11, 2016

Member

@rowanmiller I talked to @natemcmaster and we think we should take a fix for this now.

Member

divega commented May 11, 2016

@rowanmiller I talked to @natemcmaster and we think we should take a fix for this now.

@divega divega added this to the 1.0.0-rc2 milestone May 11, 2016

@divega divega added the type-bug label May 11, 2016

@divega divega added the pri0 label May 11, 2016

@divega

This comment has been minimized.

Show comment
Hide comment
@divega

divega May 11, 2016

Member

FWIW the rationale for taking this now is that:

  1. The PMC commands are the tooling option that works for the greatest set of scenarios in RC2
  2. I believe the user gets a very unhelpful exception
  3. PowerShell 5.0 requires extra downloads for Windows 7, 8, 8.1 and the corresponding server SKUs.
Member

divega commented May 11, 2016

FWIW the rationale for taking this now is that:

  1. The PMC commands are the tooling option that works for the greatest set of scenarios in RC2
  2. I believe the user gets a very unhelpful exception
  3. PowerShell 5.0 requires extra downloads for Windows 7, 8, 8.1 and the corresponding server SKUs.
@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller May 11, 2016

Member

Sounds fine to me... should be a low risk fix

Member

rowanmiller commented May 11, 2016

Sounds fine to me... should be a low risk fix

natemcmaster added a commit that referenced this issue May 11, 2016

natemcmaster added a commit that referenced this issue May 11, 2016

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster

natemcmaster May 11, 2016

Member

To be clear, this only affects users on *.xproj. PMC commands for csproj are unaffected by this.

Member

natemcmaster commented May 11, 2016

To be clear, this only affects users on *.xproj. PMC commands for csproj are unaffected by this.

@rowanmiller rowanmiller modified the milestones: 1.0.0, 1.0.0-rc2 May 11, 2016

@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller May 11, 2016

Member

Spoke with @danroth27, this does not meet the bar to reset RC2 builds. Moving to RTM.

Member

rowanmiller commented May 11, 2016

Spoke with @danroth27, this does not meet the bar to reset RC2 builds. Moving to RTM.

@ErikEJ

This comment has been minimized.

Show comment
Hide comment
@ErikEJ

ErikEJ May 11, 2016

Contributor

So Windows 10 or manual changes to the psm1 file will be required in rc2?

Contributor

ErikEJ commented May 11, 2016

So Windows 10 or manual changes to the psm1 file will be required in rc2?

@natemcmaster

This comment has been minimized.

Show comment
Hide comment
@natemcmaster
Member

natemcmaster commented May 11, 2016

@rowanmiller

This comment has been minimized.

Show comment
Hide comment
@rowanmiller

rowanmiller May 11, 2016

Member

Upgrading PS would be our guidance over modifying the scripts

Member

rowanmiller commented May 11, 2016

Upgrading PS would be our guidance over modifying the scripts

@ErikEJ

This comment has been minimized.

Show comment
Hide comment
@ErikEJ

ErikEJ May 11, 2016

Contributor

Of course!

Contributor

ErikEJ commented May 11, 2016

Of course!

@divega

This comment has been minimized.

Show comment
Hide comment
@divega
Member

divega commented May 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment