Replace escape_locations with escape_locations_and_make_variables everywhere

[fmeum]

This fixes #857 fundamentally by adressing the root cause - expand_locations can't be applied to a shell command fragment by itself as it doesn't unescape $$.

I divided this PR into individual commits with more detailed commit messages to make the review easier, but feel free to squash them in the end if that's your preferred workflow.

[fmeum]

The RBE job fails with

Output type mismatch
--
  | java.io.IOException: io.grpc.StatusRuntimeException: FAILED_PRECONDITION: Output type mismatch
  | at com.google.devtools.build.lib.remote.GrpcRemoteExecutor.executeRemotely(GrpcRemoteExecutor.java:235)
  | at com.google.devtools.build.lib.remote.RemoteExecutionService.executeRemotely(RemoteExecutionService.java:1243)
  | at com.google.devtools.build.lib.remote.RemoteSpawnRunner.lambda$exec$2(RemoteSpawnRunner.java:264)
  | at com.google.devtools.build.lib.remote.Retrier.execute(Retrier.java:244)
  | at com.google.devtools.build.lib.remote.RemoteRetrier.execute(RemoteRetrier.java:125)
  | at com.google.devtools.build.lib.remote.RemoteRetrier.execute(RemoteRetrier.java:114)
  | at com.google.devtools.build.lib.remote.RemoteSpawnRunner.exec(RemoteSpawnRunner.java:239)
  | at com.google.devtools.build.lib.exec.SpawnRunner.execAsync(SpawnRunner.java:245)
  | at com.google.devtools.build.lib.exec.AbstractSpawnStrategy.exec(AbstractSpawnStrategy.java:146)
  | at com.google.devtools.build.lib.exec.AbstractSpawnStrategy.exec(AbstractSpawnStrategy.java:108)
  | at com.google.devtools.build.lib.actions.SpawnStrategy.beginExecution(SpawnStrategy.java:47)
  | at com.google.devtools.build.lib.exec.SpawnStrategyResolver.beginExecution(SpawnStrategyResolver.java:68)
  | at com.google.devtools.build.lib.analysis.actions.SpawnAction.beginExecution(SpawnAction.java:328)
  | at com.google.devtools.build.lib.actions.Action.execute(Action.java:133)
  | at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor$5.execute(SkyframeActionExecutor.java:907)
  | at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor$ActionRunner.continueAction(SkyframeActionExecutor.java:1076)
  | at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor$ActionRunner.run(SkyframeActionExecutor.java:1031)
  | at com.google.devtools.build.lib.skyframe.ActionExecutionState.runStateMachine(ActionExecutionState.java:152)
  | at com.google.devtools.build.lib.skyframe.ActionExecutionState.getResultOrDependOnFuture(ActionExecutionState.java:91)
  | at com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.executeAction(SkyframeActionExecutor.java:492)
  | at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.checkCacheAndExecuteIfNeeded(ActionExecutionFunction.java:856)
  | at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.computeInternal(ActionExecutionFunction.java:349)
  | at com.google.devtools.build.lib.skyframe.ActionExecutionFunction.compute(ActionExecutionFunction.java:169)
  | at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:590)
  | at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:382)
  | at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
  | at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
  | at java.base/java.lang.Thread.run(Unknown Source)
  | Caused by: io.grpc.StatusRuntimeException: FAILED_PRECONDITION: Output type mismatch
  | at io.grpc.Status.asRuntimeException(Status.java:535)
  | at io.grpc.stub.ClientCalls$BlockingResponseStream.hasNext(ClientCalls.java:648)
  | at com.google.devtools.build.lib.remote.GrpcRemoteExecutor.lambda$executeRemotely$2(GrpcRemoteExecutor.java:169)
  | at com.google.devtools.build.lib.remote.Retrier.execute(Retrier.java:244)
  | at com.google.devtools.build.lib.remote.RemoteRetrier.execute(RemoteRetrier.java:125)
  | at com.google.devtools.build.lib.remote.RemoteRetrier.execute(RemoteRetrier.java:114)
  | at com.google.devtools.build.lib.remote.GrpcRemoteExecutor.lambda$executeRemotely$3(GrpcRemoteExecutor.java:140)
  | at com.google.devtools.build.lib.remote.util.Utils.refreshIfUnauthenticated(Utils.java:525)
  | at com.google.devtools.build.lib.remote.GrpcRemoteExecutor.executeRemotely(GrpcRemoteExecutor.java:138)
  | ... 27 more

That does look too generic to be caused by this PR, but I'm not sure. @UebelAndre Have you seen this before?

[garrettkajmowicz-sophos]

I tested this locally and it seems to address the issue as well.

[jsharpe]

@fmeum I've just merged a PR to pin buildifier to the previous version which should resolve the build failures here on a rebase

[attilaolah]

It would be great if you we could push a new release including this PR, as I have some failing builds (that were passing in an ancient version using some other workarounds), and resolving those failures requires this change to be merged.

[UebelAndre]

It would be great if you we could push a new release including this PR, as I have some failing builds (that were passing in an ancient version using some other workarounds), and resolving those failures requires this change to be merged.

Can you create a new issue requesting the release so we can track it better?