Skip to content
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

Start-Job needs a -WorkingDirectory parameter #4287

Closed
mklement0 opened this issue Jul 18, 2017 · 9 comments · Fixed by #10324
Closed

Start-Job needs a -WorkingDirectory parameter #4287

mklement0 opened this issue Jul 18, 2017 · 9 comments · Fixed by #10324
Labels
Hacktoberfest Potential candidate to participate in Hacktoberfest Issue-Enhancement the issue is more of a feature request than a bug Resolution-Fixed The issue is fixed. Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors WG-Cmdlets-Core cmdlets in the Microsoft.PowerShell.Core module
Milestone

Comments

@mklement0
Copy link
Contributor

mklement0 commented Jul 18, 2017

Currently, Start-Job:

Either way, there is no simple way to have the caller set the working directory explicitly, leading to such painful workarounds as in this SO answer.

The proposed solution:

# Wishful thinking
> $jb = Start-Job -WorkingDirectory $PSHOME { "Hi from $PWD." }; Receive-Job -AutoRemove -Wait $jb
Hi from C:\Program Files\PowerShell\6.0.0-beta.4

Environment data

PowerShell Core v6.0.0-beta.4 on macOS 10.12.5
PowerShell Core v6.0.0-beta.4 on Ubuntu 16.04.2 LTS
PowerShell Core v6.0.0-beta.4 on Microsoft Windows 10 Pro (64-bit; v10.0.15063)
Windows PowerShell v5.1.15063.413 on Microsoft Windows 10 Pro (64-bit; v10.0.15063)
@SteveL-MSFT SteveL-MSFT added WG-Cmdlets-Core cmdlets in the Microsoft.PowerShell.Core module Issue-Enhancement the issue is more of a feature request than a bug Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors labels Jul 18, 2017
@SteveL-MSFT SteveL-MSFT added this to the 6.1.0 milestone Jul 18, 2017
@lzybkr
Copy link
Member

lzybkr commented Jul 25, 2017

If this gets added, then the language feature & should use it instead of how it is currently implemented.

@Average-Bear
Copy link

Would like to see Start-Job have -ThrottleLimit compatibility.

@mklement0
Copy link
Contributor Author

@Average-Bear: I suggest you create a new issue (and provide a rationale for your request there).

@IISResetMe
Copy link
Collaborator

WorkingDirectory is a property specific to a process - and the fact that Start-Job spins up an out-of-process runspace seems to me an implementation detail, not something you would necessarily want to bring up to the level of abstraction that a Job provides.

Seems like the kind of thing you would want to include in the initialization script, so I'd recommend just fixing #4530 so you can do:

$jb = Start-Job { "Hi from $PWD." } -InitializationScript {Set-Location $using:PWD}; Receive-Job -AutoRemove -Wait $jb

rather than the currently super awkward:

$jb = Start-Job { "Hi from $PWD." } -InitializationScript ([scriptblock]::Create("Set-Location $PWD")); Receive-Job -AutoRemove -Wait $jb

@mklement0
Copy link
Contributor Author

I consider -InitializationScript {Set-Location $using:PWD} awkward too.

Having a -WorkingDirectory is a matter of convenience first and foremost, and it also provides symmetry with Start-Process.

You're running a script block / script somewhere, and it's helpful to have a simple way to control that somewhere.

This is especially true with the current behavior, where you - invisibly - run in a location other than the current one (unlike when you use the new & operator on Unix - a regrettable discrepancy - see #4267).

As an aside, re implementation detail: Understanding the underpinnings of jobs is important, because users need to be aware that a separate process and remoting are involved to understand that deserialized objects are returned.

@SteveL-MSFT SteveL-MSFT modified the milestones: 6.1.0-Consider, Future Jun 20, 2018
@davinci26
Copy link
Contributor

@mklement0 @SteveL-MSFT

I want to take a stab at this one and before deep diving into the implementation I wanted to see what you think about the following solutions:

  1. Quick Solution that respects the Job abstraction:
1. Get the working directory from the user.
2. Verify that the directory exists
3. Inject in the beginning of the script block that will be executed a 'Set-Location $UserSpecifiedWorkingDirectory'
  1. Propagate the $WorkingDirectory variable all the way up to the Job level and specify the working directory there when the process starts

What do you think? Do any of these approaches seem reasonable?

Disclaimer: this is my first issue here so I might be missing something entirely

@KirkMunro
Copy link
Contributor

@davinci26 I wouldn't inject anything into the script block. That could result in some surprises if/when you debug a job (i.e. you should only see your job script when you debug a job, not extra stuff that your job script didn't include). Instead I'd just set the location when the runspace associated with the job is created, before the script block is run inside of it.

@davinci26
Copy link
Contributor

I took a deep dive at the codebase and I observed the following:

Creating a new pipeline with a Set-Location and adding it to the operations list:

protected override void CreateHelpersForSpecifiedComputerNames()

Just add this

var command = new Command("Set-Location");
command.Parameters.Add("LiteralPath", this.WorkingDirectory);
Pipeline tempPipeline = remoteRunspace.CreatePipeline(command.ToString());
tempPipeline.Commands.Add(command);
IThrottleOperation locationOperation = new ExecutionCmdletHelperComputerName(remoteRunspace, tempPipeline);
Operations.Add(locationOperation);

If you follow this approach then pwsh throws because the second operation in Operations is trying to open a runspace that is already open (see here)

Is this the intended behaviour? Can we modify the logic when we try to open the remote the runspace to skip the opening if the runspace is already open.

Adding a Set-Location command to the pipeline

This would require to either:

  1. Use the CreatePipeline function available in PSRemotingCmdlets and then insert the command in the beginning. Personally I am not a huge fan of this approach as it would be a bit slow.

  2. Modify the CreatePipeline function directly.

internal Pipeline CreatePipeline(RemoteRunspace remoteRunspace)

This function is consumed by all PSRemoteCmdlets. Would the workingDirectory parameter make sense in all of them? Is this an overkill?

Changing the working directory of the pwsh remote server:

  1. The command line argument workingdirectory does not work when the pwsh process runs in server mode (-s flag).
  2. The InitializationScript argument is passed to the process startupInfo as part of the arguments.

If we stick to the implementation of (1) & (2) I do not see how we could have a workingDirectory parameter for Start-Job that would be able to set the working directory also for the InitializationScript. Is this part of the requirement?

Enabling (1) would allow us to implement the working directory fairly easily since we can add an additional command line argument to when the powershell server process instance is spawned.

@ghost
Copy link

ghost commented Sep 19, 2019

🎉This issue was addressed in #10324, which has now been successfully released as v7.0.0-preview.4.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest Potential candidate to participate in Hacktoberfest Issue-Enhancement the issue is more of a feature request than a bug Resolution-Fixed The issue is fixed. Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors WG-Cmdlets-Core cmdlets in the Microsoft.PowerShell.Core module
Projects
None yet
8 participants