Enable discovering modules that have names same as a culture (e.g. Az)#8777
Enable discovering modules that have names same as a culture (e.g. Az)#8777daxian-dbw merged 6 commits intoPowerShell:masterfrom
Conversation
4314122 to
3ed7768
Compare
|
The static analysis failure is not related to this PR. I'll submit a fix for it in a separate PR. |
|
The PR description is updated as we are not using recursion when searching for modules. |
|
Could someone help me understand the delivery vehicle and timing for this fix? Is this a fix for Windows PowerShell 5.1 or for PowerShell Core (6.0)? If Windows PowerShell 5.1, would that mean delivery would be part of a 5.2 type release? Are consumers today completely blocked from building module dependencies on the new Az module until this is resolved? |
|
@ronhowe The fix is only for PowerShell Core. You'll get it in PowerShell Core 6.2 RC in hours and in release PowerShell Core 6.2 in weeks. Windows PowerShell 5.2 is not expected. Although MSFT may port this fix to 5.1.
The problem is only for auto discovering. You could use full path to explicitly load the module. |
|
@ronhowe: to reiterate what @iSazonov said, our bar for servicing Windows PowerShell is very high now, and we do not plan to do a 5.2 release. This has resulted in a small handful of security and accessibility fixes, and some bug fixes to fix regressions associated with those two. That being said, I want to talk to the Azure PowerShell team about this (/cc @sphibbs and @markcowl for context), as I don't want us limiting the adoption of In the meantime, using Apparently, it may also be the case that But like I said, I'll follow up with the Azure PS to get their sense of the impact. Pure curiosity and off-topic, but I'd also love to hear what's keeping you from using PowerShell Core. :) |
|
Dove a little deeper on this...@ronhowe: what exactly is the problem you're experiencing? We've confirmed that Additionally, because So again, how are you actually being affected here? |
I just read this and that's the exact problem. I can try to be more selective about what my module imports. |
|
As to why we aren't using PowerShell Core? I'm all for that. It's a bit more difficult to spearhead that adoption across my company. I am at the frontline of that effort, but it's a much slower ship and we have more immediate needs. |
|
The impact is that the user is given the With everyone writing their own module install\update logic, we have to walk the |
|
You'd think that the Azure team would be verifying that the module works as expected in a majority of use cases before finalizing a module structure that is actually broken on both the supposedly supported platforms. I guess for whatever reason... these cases were overlooked. I don't know that I could fault the PS team for this behaviour moreso than the Azure team for not checking that the module functions as expected with the structure they chose. :/ |
|
Both teams are failing ME - the user. |
PR Summary
The suggested fix by @lzybkr is to not do this additional culture name check for the first set of folders (which should be the module name). This PR fixes a few things:
EnumerationOptionto skip hidden folders.IsPossibleModuleDirectory()helper now only checks to see if the name matches a culture, renamed toIsPossibleResourceDirectory()PR Context
The current code always checks if a potential module folder is named after a culture and assumes it contains resources files as an optimization to avoid looking in that folder. This results in the new Az module not being found as
Azis also the name of a culture.Fix #8125
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.[feature]to your commit messages if the change is significant or affects feature tests