GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Exclude -Comobject parameter of New-Object cmdlet on unsupported platforms by iSazonov · Pull Request #4922 · PowerShell/PowerShell

Conversation

Ilya (iSazonov) commented Sep 26, 2017

Copy link

Copy Markdown

Collaborator

Fix #4897.

New-Object -Comobject is not supported on

Linux and MacOs - fix is remove the code.
Windows Core and IoT - fix is throw NotSupportedException exception.

First commit reformat test file.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Convert indentations to spaces in tests

8895ea0

Ilya (iSazonov) assigned Dongbo Wang (daxian-dbw)

Sep 26, 2017

Ilya (iSazonov) requested review from James Truher (JamesWTruher), Paul Higinbotham (PaulHigin) and Dan Travison (dantraMSFT) as code owners

September 26, 2017 13:38

Ilya (iSazonov) added 2 commits

September 26, 2017 16:47


          Exclude on Unix ComObject parameter in New-Object cmdlet

9fa0274


          Add tests

baf780f

Ilya (iSazonov) force-pushed the comobject-exclude branch from c94c454 to baf780f Compare

September 26, 2017 13:47

Dongbo Wang (daxian-dbw) requested changes

Sep 26, 2017

View reviewed changes

Comment thread

src/Microsoft.PowerShell.Commands.Utility/commands/utility/new-object.cs Outdated

		{
		if (!Platform.IsWindowsDesktop)
		{
		Exception exc = new NotSupportedException(NewObjectStrings.ComObjectPlatformIsNotSupported);

Dongbo Wang (daxian-dbw) Sep 26, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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.

COM works on NanoServer as well, as long as it has a workable COM component
PS: the COM support was enabled when powershell core was still NanoServer powershell.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Sep 26, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator Author

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.

I based on current tests. 😕
So should we exclude the parameter only on Unix?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Dongbo Wang (daxian-dbw) Sep 26, 2017

Copy link

Copy Markdown

Member

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.

The COM components we use in our tests are not available in NanoServer and IoT.
Yes, we should exclude the parameter only for Unix plats.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Ilya (iSazonov) Sep 27, 2017

Copy link

Copy Markdown

Collaborator Author

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.

Does "Shell.Application" work on NanoServer and IoT? can I use this in a test?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Dongbo Wang (daxian-dbw) Sep 27, 2017

Copy link

Copy Markdown

Member

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.

Shell itself is not available on NanoServer and IoT. You can use it as a test, but just follow the existing tests in test\powershell\engine\COM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Sep 27, 2017

Copy link

Copy Markdown

Collaborator Author

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.

Why shouldn't we targeting our test to NanoServer and IoT?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) suggested changes

Sep 26, 2017

View reviewed changes

Comment thread

src/Microsoft.PowerShell.Commands.Utility/commands/utility/new-object.cs

		}

		#endif
		#endregion Com

Paul Higinbotham (PaulHigin) Sep 26, 2017

Copy link

Copy Markdown

Contributor

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.

I think the entire "com" region here should be under the #if !UNIX.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Sep 27, 2017

Copy link

Copy Markdown

Collaborator Author

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.

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) force-pushed the comobject-exclude branch from 50a3c63 to 84f7a6c Compare

September 27, 2017 04:40


          Exclude Comobject only on Unix

2af645b

Ilya (iSazonov) force-pushed the comobject-exclude branch from 84f7a6c to 2af645b Compare

September 27, 2017 04:53

Paul Higinbotham (PaulHigin) approved these changes

Sep 27, 2017

View reviewed changes

Paul Higinbotham (PaulHigin) left a comment

Copy link

Copy Markdown

Contributor

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Dongbo Wang (daxian-dbw) requested changes

Sep 27, 2017

View reviewed changes

Comment thread

test/powershell/Modules/Microsoft.PowerShell.Utility/New-Object.Tests.ps1 Outdated

		if ($IsLinux -or $IsMacOs ) {
		{ New-Object -ComObject "Shell.Application" } \| ShouldBeErrorId "NamedParameterNotFound,Microsoft.PowerShell.Commands.NewObjectCommand"
		} else {
		{ New-Object -ComObject "Shell.Application" } \| Should Not Throw

Dongbo Wang (daxian-dbw) Sep 27, 2017

Copy link

Copy Markdown

Member

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.

This will fail when running tests on NanoServer ... Maybe change it to check if New-Object has the parameter ComObject instead of creating the Shell.Application object?

$s = gcm New-Object
$s.Parameters.ContainsKey("ComObject")
True

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction


          Correct test to support NamoServer and IoT

24b9545

Dongbo Wang (daxian-dbw) approved these changes

Sep 28, 2017

View reviewed changes

Dongbo Wang (daxian-dbw) merged commit b07f24e into PowerShell:master

Sep 29, 2017

Ilya (iSazonov) deleted the comobject-exclude branch

September 29, 2017 03:29

Ilya (iSazonov) mentioned this pull request

Nov 16, 2018

Remove Persist parameter from New-PSDrive on non-Windows platform #8291

Merged

8 of 11 tasks

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Exclude '-Comobject' parameter of 'New-Object' on unsupported platfor…

4c29e9e

…ms (PowerShell#4922)

Reviewers

Dongbo Wang (daxian-dbw)

daxian-dbw approved these changes

Dan Travison (dantraMSFT)

Awaiting requested review from dantraMSFT

James Truher (JamesWTruher)

Awaiting requested review from JamesWTruher

+1 more reviewer

Paul Higinbotham (PaulHigin)

PaulHigin approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Dongbo Wang (daxian-dbw)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

-ComObject should not be exposed on systems where it is not supported

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Ilya (iSazonov) commented Sep 26, 2017

Uh oh!

Dongbo Wang (daxian-dbw) Sep 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Sep 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Paul Higinbotham (PaulHigin) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Dongbo Wang (daxian-dbw) Sep 26, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) Sep 26, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.