Skip to content

Conversation

@PaulHigin
Copy link
Contributor

PR Summary

This fixes a bug in ForEach-Object -Parallel where a using variable in a nested ForEach-Object -Parallel throws an error even when the variable is defined in the correct scope (Issue #11817).

PR Context

This error was occurring because, when ForEach-Object -Parallel was assembling the user variable map, it was searching all nested scriptblocks within the ForEach scriptblock, with the result of finding nested using variables where the variable had not yet been defined. Since the nested scriptblock using variable had not been defined in the current scope, a mapping error was thrown.

Fix is to change the using variable map function to not search nested scriptblocks in the ForEach -Parallel case. This way foreach -parallel using variable mapping is always performed only for the current scope.

Many thanks to @mklement0 for pointing out the fix.

PR Checklist

@iSazonov iSazonov added CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log BackPort-7.1.x-Consider labels Jan 6, 2021
@PaulHigin
Copy link
Contributor Author

@daxian-dbw Can you please review these changes?

@PaulHigin PaulHigin requested a review from SteveL-MSFT January 12, 2021 17:14
Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

One comment, otherwise looks fine to me

@ghost ghost added the Review - Needed The PR is being reviewed label Jan 21, 2021
@ghost
Copy link

ghost commented Jan 21, 2021

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@anmenaga
Copy link

Restarted CIs for what looks like unrelated failures in Update-Help tests.

@ghost ghost removed the Review - Needed The PR is being reviewed label Jan 28, 2021
@anmenaga
Copy link

@PaulHigin it would be good to fix CodeFactor violations in the new code that is added; thank you.

@anmenaga
Copy link

anmenaga commented Feb 1, 2021

Restarted CIs again.

@anmenaga
Copy link

anmenaga commented Feb 2, 2021

One more time restarted CIs.

@anmenaga anmenaga merged commit 0039807 into PowerShell:master Feb 2, 2021
@daxian-dbw daxian-dbw added this to the 7.2.0-preview.3 milestone Feb 2, 2021
@ghost
Copy link

ghost commented Feb 12, 2021

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

Handy links:

@adityapatwardhan
Copy link
Member

@PaulHigin - Should this be backported to 7.1.x?

@PaulHigin
Copy link
Contributor Author

@adityapatwardhan I think the impact of this change is small, since if involves nested foreach calls. So I would say it does not need to be backported as it probably does not affect a lot of users.

@adityapatwardhan
Copy link
Member

Removed the back port label as per comment from @PaulHigin

@keystroke
Copy link

This is not unique to parallel foreach, it applies to nested start-job and nested invoke-command calls (double-hop) or any combination of those, like using invoke-command and then running start-job in the remote session, etc. Does this fix work for these scenarios as well (everywhere $using is supported) or was this somehow scoped to parallel foreach?

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

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants