Skip to content

Set correct PSProvider full name - #11813

Merged
Aditya Patwardhan (adityapatwardhan) merged 7 commits into
PowerShell:masterfrom
iSazonov:fix-provider-drive-hidden2
Apr 27, 2020
Merged

Set correct PSProvider full name#11813
Aditya Patwardhan (adityapatwardhan) merged 7 commits into
PowerShell:masterfrom
iSazonov:fix-provider-drive-hidden2

Conversation

@iSazonov

@iSazonov Ilya (iSazonov) commented Feb 8, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

PR Summary

Address #9840.

  • Set correct PSProvider full name at module load time.
  • Make PSProvider.FullName public

PR Context

In #8831 we added caching for ProviderInfo.FullName. This created a side effect.
PowerShell engine loads a module with name based on RootModule manifest property. Then it set explicitly correct module name based on the module (manifest) path. If the module contains a provider PowerShell engine loads and initializes the provider too. Full name of the provider is based on the module name and it is cached. So we should update the provider full name too that the fix explicitly does.

PR Checklist

@iSazonov Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Feb 8, 2020
@iSazonov Ilya (iSazonov) added this to the GA-consider milestone Feb 8, 2020
@iSazonov Ilya (iSazonov) mentioned this pull request Feb 8, 2020
Closed
14 tasks
PoshChan
PoshChan-Bot (PoshChan) reviewed Feb 8, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/DataStoreAdapterProvider.cs Outdated
PoshChan
PoshChan-Bot (PoshChan) reviewed Feb 8, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/DataStoreAdapterProvider.cs Outdated
PoshChan
PoshChan-Bot (PoshChan) reviewed Feb 8, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/DataStoreAdapterProvider.cs Outdated
@iSazonov
Ilya (iSazonov) force-pushed the fix-provider-drive-hidden2 branch from e9511dd to 5230d80 Compare February 8, 2020 17:15
PoshChan
PoshChan-Bot (PoshChan) reviewed Feb 8, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/DataStoreAdapterProvider.cs Outdated
PoshChan
PoshChan-Bot (PoshChan) reviewed Feb 8, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/DataStoreAdapterProvider.cs Outdated
PoshChan
PoshChan-Bot (PoshChan) reviewed Feb 8, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/DataStoreAdapterProvider.cs Outdated
rjmholt
Rob Holt (rjmholt) reviewed Feb 8, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs Outdated

@rjmholt Rob Holt (rjmholt) Feb 8, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Given that you're using a foreach immediately below, would it be possible to remove the LINQ usage altogether?

@iSazonov Ilya (iSazonov) Feb 9, 2020

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I removed the LINQ.

PoshChan
PoshChan-Bot (PoshChan) reviewed Feb 9, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/DataStoreAdapterProvider.cs Outdated
rjmholt
Rob Holt (rjmholt) approved these changes Feb 9, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs Outdated
@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Feb 10, 2020

Copy link
Copy Markdown
Member

Ilya (@iSazonov) Adding a new public API will need extra scrutiny for considering this for 7 GA.

@iSazonov

Ilya (iSazonov) commented Feb 11, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Aditya Patwardhan (@adityapatwardhan) I reverted FullName to internal and update test.

daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Feb 11, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs Outdated
@ghost ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Feb 11, 2020
@iSazonov

Ilya (iSazonov) commented Mar 13, 2020

Copy link
Copy Markdown
Collaborator Author

Dongbo Wang (@daxian-dbw) Rob Holt (@rjmholt) Could you please review?

@iSazonov
Ilya (iSazonov) force-pushed the fix-provider-drive-hidden2 branch from 279cfcf to e3ed09b Compare March 29, 2020 06:46
@iSazonov

Ilya (iSazonov) commented Mar 29, 2020

Copy link
Copy Markdown
Collaborator Author

Dongbo Wang (@daxian-dbw) Could you please continue?

@adityapatwardhan

Aditya Patwardhan (adityapatwardhan) commented Apr 8, 2020

Copy link
Copy Markdown
Member

Dongbo Wang (@daxian-dbw) Please re-review

daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Apr 24, 2020
View reviewed changes
Comment thread src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Apr 24, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we probably should add the smart-ness into ProviderInfo.FullName to change the cached _fullName when the ModuleName property is changing.
Something like this:

private string _cachedModuleName = null;
internal string FullName
{
    get
    {
        string GetFullName(string name, string psSnapInName, string moduleName)
        { ... }

        if (_fullName != null && _cachedModuleName == ModuleName)
        {
            return fullName;
        }

        _cachedModuleName = ModuleName;
        return (_fullName = GetFullName(Name, PSSnapInName, ModuleName));
    }
}

@iSazonov Ilya (iSazonov) Apr 25, 2020

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Apr 24, 2020
@iSazonov
Ilya (iSazonov) force-pushed the fix-provider-drive-hidden2 branch from e3ed09b to 47e0d30 Compare April 25, 2020 07:02
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Apr 25, 2020
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Apr 26, 2020
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@adityapatwardhan
Aditya Patwardhan (adityapatwardhan) merged commit 86b6a9d into PowerShell:master Apr 27, 2020
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) added CL-Engine Indicates that a PR should be marked as an engine change in the Change Log and removed CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log labels Apr 27, 2020
@iSazonov
Ilya (iSazonov) deleted the fix-provider-drive-hidden2 branch April 28, 2020 03:23
@ghost

Deleted user (ghost) commented May 19, 2020

Copy link
Copy Markdown

🎉v7.1.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

@rjmholt Rob Holt (rjmholt) mentioned this pull request Mar 2, 2021
Merged
22 tasks
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
@iSazonov
Set correct PSProvider full name (PowerShell#11813)
2fae22c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) adityapatwardhan approved these changes
@rjmholt Rob Holt (rjmholt) Awaiting requested review from rjmholt
+1 more reviewer
@PoshChan PoshChan-Bot (PoshChan) PoshChan left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan Aditya Patwardhan (adityapatwardhan)

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.3

Development

Successfully merging this pull request may close these issues.

6 participants

@iSazonov @adityapatwardhan @daxian-dbw @rjmholt @PoshChan @TravisEz13