pipeTransport.pipeArgs command variable replacement not working

[davidruhmann]

Attempting to figure out why input and command replacements do not work for launch.json pipeTransport.pipeArgs (used in omnisharp-csharp extension). My use case requires omnisharp-vscode extension. However, this involves the vscode variable replacement logic, so I am posting it here.

• VSCode Version: 1.41.1 (also tried insider 1.42.0-insider)
• OS Version: Darwin x64 18.7.0

Steps to Reproduce:

1. Create a launch.json for remote debugging a csharp docker container. (e.g. vsdbg)

My Use Case Example (requires vscode-docker extension)

{
    "version": "0.2.0",
    "configurations": [
        {
            "name": ".NET Core Docker Attach",
            "type": "coreclr",
            "request": "attach",
            "pipeTransport": {
                "pipeProgram": "docker",
                "pipeArgs": [
                    "exec", "-i", "${command:vscode-docker.containers.select}"
                ],
                "debuggerPath": "/root/vsdbg/vsdbg",
                "pipeCwd": "${workspaceRoot}",
                "quoteArgs": false
            },
            "processId": "${command:pickRemoteProcess}",
            "sourceFileMap": {
                "/app": "${workspaceRoot}"
            },
            "justMyCode": true
        }
    ]
}

vscode-docker.containers.select reference: microsoft/vscode-docker#1240

Another Example

{
    "version": "0.2.0",
    "configurations": [
        {
            "name": ".NET Core Docker Attach",
            "type": "coreclr",
            "request": "attach",
            "pipeTransport": {
                "pipeProgram": "docker",
                "pipeArgs": [
                    "exec", "-i", "${input:sampleInput}"
                ],
                "debuggerPath": "/root/vsdbg/vsdbg",
                "pipeCwd": "${workspaceRoot}",
                "quoteArgs": false
            },
            "processId": "${command:pickRemoteProcess}",
            "sourceFileMap": {
                "/app": "${workspaceRoot}"
            },
            "justMyCode": true
        }
    ],
    "inputs": [
        {
            "id": "sampleInput",
            "type": "promptString",
            "description": "issue example"
        }
    ]
}

2. It will display the vscode prompts and dialogs, but not actually replace the entered/command value into the string for either example.

Output

Executing: docker exec -i ${input:sampleInput} sh -s < /Users/user/.vscode/extensions/ms-vscode.csharp-1.21.9/scripts/remoteProcessPickerScript
stderr: "docker exec" requires at least 2 arguments.
...

Does this issue occur when all extensions are disabled?: Not Sure.

[vscodebot]

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

• Dependent commands within launch.json (#80219)

[gjsjohnmurray]

/duplicate of #85206. My understanding is that a new API lands in 1.42 that will enable debuggers to handle this situation properly.

[vscodebot]

Thanks for creating this issue! We figured it's covering the same as another one we already have. Thus, we closed this one as a duplicate. You can search for existing issues here. See also our issue reporting guidelines.

Happy Coding!

[davidruhmann]

@gjsjohnmurray Thank you for the info!

If I understand this correctly, omnisharp-vscode extension will need to make a code change to utilize the new hook, resolveDebugConfigurationWithSubstitutedVariables , once released?

[gjsjohnmurray]

@davidruhmann yes that's my understanding too. Maybe check that project's issues to see if they're already aware of this, and open an issue if not.

[davidruhmann]

After more investigation, resolveDebugConfigurationWithSubstitutedVariables does not work for this use case as both commands are resolved/executed before replacement occurs. ${command:pickRemoteProcess} needs the value from ${command:vscode-docker.containers.select} in the config object passed to it during execution.

This issue is not a duplicate and should be reconsidered.

Created PR #92697

[weinand]

I'm opening this issue to discuss an unresolved problem that remains even after the fix for #85206.

The remaining problem:

Today variable substitution is a two phase process:

• Phase 1: the value of all variables is determined.
• Phase 2: all variables are substituted with their new values.

If a variable is command-based (e.g. relying on some code running in an extension), the extension code has only access to the unresolved launch configuration, that means it does not see other already resolved variables. The reason for this behavior is isolation and independence from evaluation order.

A consequence of this is that a launch configuration can not be considered a "sequential program", that is executed from top to bottom.

I firmly believe, that this approach is a "good design" even if it has limitations.

If I understand the PR #92697 from @davidruhmann correctly, then he wants to change variable substitution to this:

• determine the value of the first/next variable
• substitute all occurrences of that variable with the new value
• continue with the next variable

If a variable is command-based and has access to the (partially) resolved launch config, then its value becomes order dependent.

This is a problem, because we cannot preserve the order of properties in a JSON because we are not passing a launch.json as a string but as a Javascript object. The order of attributes might deviate from the order a user sees in the JSON's source (e.g. the launch configuration).

[davidruhmann]

@weinand Good breakdown.

You might want to update this comment.
#80219 (comment)

---

Just not to be lost, if at the end, the decision is to leave the behavior as is, I propose that we update the documentation to clarify how variables are resolved. Something like Variables and commands are all resolved before the values are inserted into the configuration. Order of resolution is not guaranteed.

Issues with similar confusions that I think documentation would help prevent:

• VSCode does not appear to resolve nested variables in launch.conf #86982
• after update to vscode 1.23.1, failed to attach debug  #51075 (comment)

---

The struggle I am having is figuring out the mechanism by which this use case should be solved. Appreciate feedback on them.

Here are some some ideas I have been thinking about:

• Should this use case wait for better vscode debug cli implementations so that we can invoke a debug config from the cli started by another debug config or an outside application? (Possible requires leaving the context of the application to start a debug configuration) - Suggestion: Start VSCode debugger from the command-line #10979

• Do extensions need to abandon the command mechanism for advanced use cases like this and implement their own command mechanism? Either waiting until the resolveDebugConfigurationWithSubstitutedVariables override or custom replacement keywords. This is what I tinkered around with on the omnisharp plugin before creating this PR. (Support is then determined by extension makers and this makes adaptations very explicit and limited.)

• Update the command resolution behavior in vscode.

  • Idea 1: Replace as resolved. (The PR I created in which I forgot about the json object order non-guarantee.) Though it was from your comment that I based my changes Dependent commands within launch.json #80219 (comment) ;)
  • Idea 2: Adding optional explicit order of operations to the keyword. (example: ${command[n]:dependency} where n is a number. commands at the same n level are not order guaranteed.)
  • Idea 3: Adding a depends on or order of execution array to explicitly specify order. resolutionOrder: ["command:dependency", "command:another"] // unspecified items are resolved after these.

I am still am a fan of the replacing as resolved approach. Thanks for reading through all this!

When I get a moment, may update my PR or add a new PR with an Idea 2 implementation.

Related issue and proposed solution: #80219 ( #80220)

[scabana]

As a side note, this is the same exact issue as #80219. I am the author of the vscode-docker.containers.select command 😄 The command in vscode-docker and the #80219 issue were created pretty much at the same time.

My initial suggestion to resolve this was idea 2, but @weinand questionned it back then. This commit d545dc3 shows when I reverted it. It was slightly different though: ${command:n:dependency} where :n was not mandatory. I think it would be important to have the additional : seperator.

@davidruhmann if your PR goes through and not mine, I had added a little more(not much, but still) tests than you, which could be ported back to your PR.

@weinand I think either this issue or #80219 should be closed as duplicate.

[davidruhmann]

@scabana Appreciate your information! I remember seeing your issue when investigating, but missed the PR you had made.

Your changes look nice, so if you want to add them back in for consideration that works for me.

Most of my time for these tasks has to wait for the weekends, so I probably will not be able to make any changes till then.

[scabana]

I'd rather complete the discussion on which solution to use before adding more time into a solution that would not be good enough to be considered. I also have time to implement a solution. Let's see what @weinand thinks about this. My issue was "forgotten"/put on hold for months even though there was an associated PR (I understant the number of issues/prs opened on this repo, I'm not frustrated by it). I'd like to get a solution validated before code is done.

[weinand]

Thanks for providing input and PRs for the discussion.

My current take on the issue is still what I said yesterday:
Interleaving variable resolution with substitution is non-deterministic without a way to control the order of variable resolution. This is no option because it results in hard to find errors.

Interleaving variable resolution with substitution and introducing a mechanism to specify the order (e.g. via PR #80220) addresses the problem of non-determinism, but the syntax and the need to specify an order looks a bit clumsy for my taste.

But there is a bigger issue.

The problem is that a JSONs is:

• a data structure and not a program and
• "variable substitution" is just a "macro" mechanism on top of the data structure and not the variables concept known from programming languages.

Introducing immediate substitution of variables after their resolution and introducing sequence numbers to control the order of variable tries to turn the data structure and its "macro" mechanism into a real sequential program.

But this is the wrong message for users. If we are opening the floodgates, there will be a stream of feature requests to add more program features to launch.json and task.json. Most likely the first request will be one for initialising variables before the substitution takes place...

Before we are giving in to this direction, we should really try harder to solve the issue with a different approach.

E.g. if we have dependencies between variables that rely on extension code and we cannot express the sequential evaluation order of these variables in a JSON, then maybe we should consider to use (extension) code instead.

An interesting observation is that the only issues we got for this problem originated from the "pipeTransport" construct introduced for the "coreclr" debugger.

Could we find a replacement for the "pipeTransport" complexity that moves the dependencies into the extension instead of having it in the launch.json?

[scabana]

Thanks for taking the time to elaborate.

I do agree the suggested ordering syntax is not great. I also agree that this kind of feature opens the gate for much more complex scenarios. The one thing here I'm not so sure about is the pipeTransport reference is what triggers this situation. In fact, the pipeTransport could very well be merged with the configuration (but it's still nicer to have it in a nested object, groups the values together). In the case there the pipeTransport properties would be merged into its parent, we would still have the same issue we have today for this specific scenario.

With that said, I think the real discussion here is more about the expected usage around launch.json. In the current situation, I feel, it is expected to have a lof of configurations to satisfy the current scenario. For simpler scenarios, non-dependant substitutions is good enough.

I would say that if we can't achieve dependant substitutions in vscode code base, the complexity of it should not be pushed into the extensions. The reason is that extension authors would most likely need to start looking into specific variable substitutions to know which one would run first and which one last. This would effectively bind extensions together and require way more work. Today, I don't see a way to elegantly enable this scenario without the requirement of knowing a list of commands to run first or last. I would just drop this request (and #80219).

The problem with either approaches of execution order as in ${command:n:dependency} or idea 3 is discoverability. It would not be the first example in the documentation and people would still have the same expectation of that working that I had or @davidruhmann had: this should work.

but the syntax and the need to specify an order looks a bit clumsy for my taste.

Maybe a more elegant solution would be to support a complex json object in the substitution, something like
"${ \"command\": \"command:dependency\", \"dependantOn\" = \"someotherpropertyoftheconfiguration\" }". Escaping json in a json string is not that nice, but this would make it future proof (versionnable, easier to handle older formats).

[weinand]

@scabana the problem is not the the fact that the pipeTransport is nested and merging its properties into the parent would not change anything.

The problem is that the implementation of the pickRemoteProcess relies on a fully resolved configuration being passed (which includes the nested pipeTransport).

With the current 2-pass implementation of variable substitution pickRemoteProcess sees only the unresolved configuration.

If pickRemoteProcess would delay its use of the pipeTransport to some later time (e.g. until the resolveDebugConfigurationWithSubstitutedVariables hook), it would not have the problem of some variables in the pipeTransport not being resolved.

If this is not possible (e.g. because the picker UI requires the values from the pipeTransport), then a (non-variable based) picker could be launched from the resolveDebugConfigurationWithSubstitutedVariables hook (which sees the fully resolved pipeTransport).

@WardenGnaw please chime in for your insights (since we are not yet planning to address the issue on the VS Code side).

[davidruhmann]

@weinand Just writing out the method I think you are proposing to make sure I understand.

Instead of solving this on vscode, update omnisharp-vscode to no longer use the pickRemoteProcess command face up and instead use the resolveDebugConfigurationWithSubstitutedVariables hook to run the pickRemoteProcess by some other config parameter.

Example?

{
    "version": "0.2.0",
    "configurations": [
        {
            "name": ".NET Core Docker Attach",
            "type": "coreclr",
            "request": "attach",
            "pipeTransport": {
                "pipeProgram": "docker",
                "pipeArgs": [
                    "exec", "-i", "${command:vscode-docker.containers.select}"
                ],
                "debuggerPath": "/root/vsdbg/vsdbg",
                "pipeCwd": "${workspaceRoot}",
                "quoteArgs": false
            },
            "idMethod": "pickRemoteProcess",
            "sourceFileMap": {
                "/app": "${workspaceRoot}"
            },
            "justMyCode": true
        }
    ]
}

Use something like the idMethod property for use in the coreclr resolveDebugConfigurationWithSubstitutedVariables method to invoke pickRemoveProcess in the extension and populate the processId property.

[scabana]

ok, I undertand. Been a while since I investigated this issue.

The challenge here would be to still give flexibility to the user. In this example, the pickRemoteProcess command works in a specific way, but you could think that a user would want to use another version of pickRemoteProcess that potentially comes from another extension. In this issue's scenario, it is intentional that pickRemoteProcess and vscode-docker.containers.select are not known by both extensions. Moving this deeper into the extensions would require the extensions to know that a specific command (or configured one?) would be needed to be invoked later in the pipeline. The scenario of hardcoding the container id and process id in the configuration is also valid (but I wouldn't recommend doing it).

With all that being said, from my side, we are moving away from this requirement and moving towards remote vs code. Debugging remotely is way way faster (at least of .net).

[weinand]

@davidruhmann yes, your example is exactly what I was thinking of.

And I understand that that approach results in some loss of flexibility.

But since the "pipeTransport"/"pickRemoteProcess" is the only use case where users are suffering from the limitations of the "macro-based" variable substitution mechanism, I think we could more easily fix the issue on the coreclr side than adding a new and somewhat questionable feature to VS Code's variable evaluation and substitution.

@scabana you said

moving away from this requirement and moving towards remote vscode.

I was actually hoping that VSCode Remote could be an elegant solution that would make the problem disappear.
I was already thinking about how to bring VS Code Remote into the discussion around the approach sketched by @davidruhmann. The "pickRemoteProcess" UI is a good indication that we are trying to solve a problem that is already addressed by VS Code Remote.

[WardenGnaw]

@weinand,

@Helloimbob and I were discussing this issue in microsoft/vscode-cpptools#5103 (comment)

Note: C/C++ extension and C# extension use the same pickRemoteProcess logic.

With @Helloimbob's investigation, he replaced resolveDebugConfiguration with
resolveDebugConfigurationWithSubstitutedVariables, but pickRemoteProcess still has issues.

I've now tried this with just resolveDebugConfigurationWithSubstitutedVariables defined, and it gets a slight improvement - the input prompt pops up but then in the config object passed to ShowAttachEntries() the variable is still not resolved:

Other variable types which aren't ${input:*} are resolved by this point though. Could this be a bug with VSCode and resolveDebugConfigurationWithSubstitutedVariables ?

Link to comment

Is there an issue with ${input:...} not being properly passed in resolveDebugConfigurationWithSubstitutedVariables?

[weinand]

@WardenGnaw you can find my analysis of the problem above.

This comment and this explains why we don't want to fix the issue with the submitted PRs.

In comment I try to explain how to avoid the issue.

[Helloimbob]

Is it unreasonable to suggest that ${command:...} substitution be treated as a special case and passed the partially resolved config after all other types have been resolved? And then the only problem is that you can't have one ${command:...} rely on another being resolved.

[weinand]

@Helloimbob this issue is about "the only problem" use case: one command variable depends on another command variable. Your suggestion would not solve the case that is discussed here.

And then there are the two variable types ${command:...} and ${input:...} to be special cased...

[davidruhmann]

@Helloimbob Are you proposing a change to behavior where the env and input variables all get replaced, then that config is used when evaluating the command variables?

That would solve the Another Example case in my OP.

I like this and I don't at the same time. It would be nice, but could also be confusing to those just looking at a configuration.

[Helloimbob]

@davidruhmann Yep that is what I'm proposing - you're right that it then isn't obvious that ${command:...} is evaluated later

[NapalmCodes]

I have a similar situation and stumbled upon this trying to avoid hardcoding a k8s namespace for attaching debugger to .net core:

    "version": "0.2.0",
    "configurations": [
        {
            "name": "Attach in k8s",
            "type": "coreclr",
            "request": "attach",
            "processId": "${command:pickRemoteProcess}",
            "justMyCode": false,
            "requireExactSource": false,
            "pipeTransport": {
                "pipeProgram": "kubectl",
                "pipeArgs": [
                    "exec",
                    "-i",
                    "-n",
                    "${input:namespace}",
                    "$(kubectl get pods -n",
                    "${input:namespace}",
                    "--no-headers --field-selector status.phase=Running -o custom-columns=\":metadata.name\")",
                    "--"
                ],
                "quoteArgs": false,
                "debuggerPath": "/vsdbg/vsdbg"
            },
            "sourceFileMap": {
                // For debugging you must copy source code into the app folder and point at the local folder with those
                // source items
                "/app": "${workspaceFolder}/MyService.Web"
            }
        }
    ],
    "inputs": [
        {
            "id": "namespace",
            "description": "K8S Pod Namespace:",
            "default": "my-service",
            "type": "pickString",
            "options":[
                "my-service"
            ]
        }
    ]

I thought I might be able to get around this setting pipeEnv to pass environment variables to the remote command but it doesn't appear inputs work there either or I am not using pipeEnv correctly (as it appears to be largely undocumented when searching google). Error in the terminal is /bin/sh: 1: Bad substitution

[willemodendaal]

So, trying to make sense of this... as of November 2020, is there any way to use the following as pipeArgs for pipeProgram "docker"?

"pipeArgs": ["exec","-i","${command:vscode-docker.containers.select}"],

I'm trying to write a launch configuration that can attach a debugger to a running container, but my container names are dynamically generated (I run my containers using Skaffold, so I can't control that).

My config looks like this, but doesn't work:

		{
			"name": "Attach to EXISTING Docker process",
			"type": "coreclr",
			"request": "attach",
			"processId":"${command:pickRemoteProcess}",
			"pipeTransport": {
				"pipeCwd": "${workspaceRoot}",
				"pipeProgram": "docker",
				"pipeArgs": ["exec","-i","${command:vscode-docker.containers.select}"],
				"debuggerPath": "/root/vsdbg/vsdbg",
				"quoteArgs": false
			},
			"sourceFileMap": {
				"/app": "${workspaceRoot}"
			},
			"logging": {
				"engineLogging": true
			 }
		},

The error in remote-attach output is:

Executing: docker exec -i ${command:vscode-docker.containers.select} sh -s < "c:\Users\willem\.vscode\extensions\ms-dotnettools.csharp-1.23.5\scripts\remoteProcessPickerScript"
stderr: Error: No such container: ${command:vscode-docker.containers.select}
Error Message: Command failed: docker exec -i ${command:vscode-docker.containers.select} sh -s < "c:\Users\willem\.vscode\extensions\ms-dotnettools.csharp-1.23.5\scripts\remoteProcessPickerScript"
Error: No such container: ${command:vscode-docker.containers.select}

Or is there any other way I can pass my container name to the debugger? (apart from manually editing the launch configuration every time I need to attach to a process in a container)

I tried executing the following custom .bat file as the pipeProgram, but I errors indicating that "grep", "awk" and "xargs" are not recognizable commands. Not sure how the debugger tries to execute the code.

docker ps -f name=part-of-my-container-name123 | grep "part-of" | awk "{print $1}" | xargs -n 1 -r -I CONTAINER_NAME --no-run-if-empty docker exec -it CONTAINER_NAME {process-to-execute} $1

[weinand]

As explained above (see comment and comment) variable substitution is a two phase process for good reasons.
It is in place for some time and we are no longer in a position to change variable substitution in order to accommodate the use of launch configurations as "sequential programs".

A launch.json is a configuration mechanism and variable substitution is just a string based macro mechanism.
We are aware of the limitations imposed by this, but we have no plans to address them on the VS Code side.

If these limitations mean that dependent variables can not be used in launch config, then I suggest to move the "dependent variable logic" into real code (i.e. move it into code, e.g. the extension). See my comment above.

@willemodendaal You might want to file a feature request against the C# extension.

[willemodendaal]

Thanks for clarifying @weinand