Fix running commands in Windows (#15)

README.md

@@ -22,6 +22,14 @@ Set the following in `remappings.txt`, replacing any previous definitions of the
 > **Note**
 > The above remappings mean that both `@openzeppelin/contracts/` (including proxy contracts deployed by this library) and `@openzeppelin/contracts-upgradeable/` come from your installation of the `openzeppelin-contracts-upgradeable` submodule and its subdirectories, which includes its own transitive copy of `openzeppelin-contracts` of the same release version number. This format is needed for Etherscan verification to work. Particularly, any copies of `openzeppelin-contracts` that you install separately are NOT used.
 
+### Windows installations
+
+If you are using Windows, set the `OPENZEPPELIN_BASH_PATH` environment variable to the fully qualified path of the `bash` executable.
+For example, if you are using [Git for Windows](https://gitforwindows.org/), add the following line in the `.env` file of your project (using forward slashes):
+```
+OPENZEPPELIN_BASH_PATH="C:/Program Files/Git/bin/bash"
+```
+
 ## OpenZeppelin Defender integration
 
 See [DEFENDER.md](DEFENDER.md)

src/Upgrades.sol

@@ -12,7 +12,7 @@ import {Proxy} from "@openzeppelin/contracts/proxy/Proxy.sol";
 
 import {Vm} from "forge-std/Vm.sol";
 import {console} from "forge-std/console.sol";
-import {strings} from "solidity-stringutils/strings.sol";
+import {strings} from "solidity-stringutils/src/strings.sol";
 
 import {Versions} from "./internal/Versions.sol";
 import {Utils} from "./internal/Utils.sol";
@@ -432,12 +432,20 @@ library Upgrades {
         }
 
         string[] memory inputs = _buildValidateCommand(contractName, opts, requireReference);
-        bytes memory result = Vm(CHEATCODE_ADDRESS).ffi(inputs);
+        Vm.FfiResult memory result = Utils.runAsBashCommand(inputs);
+        string memory stdout = string(result.stdout);
 
-        if (string(result).toSlice().endsWith("SUCCESS".toSlice())) {
+        // CLI validate command uses exit code to indicate if the validation passed or failed.
+        // As an extra precaution, we also check stdout for "SUCCESS" to ensure it actually ran.
+        if (result.exitCode == 0 && stdout.toSlice().contains("SUCCESS".toSlice())) {
             return;
+        } else if (result.stderr.length > 0) {
+            // Validations failed to run
+            revert(string.concat("Failed to run upgrade safety validation: ", string(result.stderr)));
+        } else {
+            // Validations ran but some contracts were not upgrade safe
+            revert(string.concat("Upgrade safety validation failed:\n", stdout));
         }
-        revert(string.concat("Upgrade safety validation failed: ", string(result)));
     }
 
     function _buildValidateCommand(

src/internal/DefenderDeploy.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.20;
 
 import {Vm} from "forge-std/Vm.sol";
 import {console} from "forge-std/console.sol";
-import {strings} from "solidity-stringutils/strings.sol";
+import {strings} from "solidity-stringutils/src/strings.sol";
 
 import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
 
@@ -19,23 +19,26 @@ library DefenderDeploy {
     using strings for *;
 
     function deploy(string memory contractName) internal returns (string memory) {
-        Vm vm = Vm(Utils.CHEATCODE_ADDRESS);
-
         string memory outDir = Utils.getOutDir();
         ContractInfo memory contractInfo = Utils.getContractInfo(contractName, outDir);
         string memory buildInfoFile = Utils.getBuildInfoFile(contractInfo.bytecode, contractInfo.shortName, outDir);
 
         string[] memory inputs = buildDeployCommand(contractInfo, buildInfoFile);
 
-        string memory result = string(vm.ffi(inputs));
-        console.log(result);
+        Vm.FfiResult memory result = Utils.runAsBashCommand(inputs);
+        string memory stdout = string(result.stdout);
+
+        if (result.exitCode == 0) {
+            console.log(stdout);
+        } else {
+            revert(string.concat("Failed to deploy contract ", contractName, ": ", string(result.stderr)));
+        }
 
         strings.slice memory delim = "Deployed to address: ".toSlice();
-        if (result.toSlice().contains(delim)) {
-            return result.toSlice().copy().find(delim).beyond(delim).toString();
+        if (stdout.toSlice().contains(delim)) {
+            return stdout.toSlice().copy().find(delim).beyond(delim).toString();
         } else {
-            // TODO extract stderr by using vm.tryFfi
-            revert(string.concat("Failed to deploy contract ", contractName, ". See error messages above."));
+            revert(string.concat("Failed to parse deployment address from output: ", stdout));
         }
     }
 

src/internal/Utils.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.8.20;
 
 import {Vm} from "forge-std/Vm.sol";
 import {console} from "forge-std/console.sol";
-import {strings} from "solidity-stringutils/strings.sol";
+import {strings} from "solidity-stringutils/src/strings.sol";
 
 struct ContractInfo {
     /**
@@ -98,8 +98,6 @@ library Utils {
         string memory contractName,
         string memory outDir
     ) internal returns (string memory) {
-        Vm vm = Vm(CHEATCODE_ADDRESS);
-
         string memory trimmedBytecode = bytecode.toSlice().beyond("0x".toSlice()).toString();
 
         string[] memory inputs = new string[](4);
@@ -108,13 +106,14 @@ library Utils {
         inputs[2] = string.concat('"', trimmedBytecode, '"');
         inputs[3] = string.concat(outDir, "/build-info");
 
-        string memory result = string(vm.ffi(inputs));
+        Vm.FfiResult memory result = runAsBashCommand(inputs);
+        string memory stdout = string(result.stdout);
 
-        if (!result.toSlice().endsWith(".json".toSlice())) {
+        if (!stdout.toSlice().endsWith(".json".toSlice())) {
             revert(string.concat("Could not find build-info file with bytecode for contract ", contractName));
         }
 
-        return result;
+        return stdout;
     }
 
     /**
@@ -183,4 +182,52 @@ library Utils {
             );
         }
     }
+
+    /**
+     * @dev Converts an array of inputs to a bash command.
+     * @param inputs Inputs for a command, e.g. ["grep", "-rl", "0x1234", "out/build-info"]
+     * @param bashPath Path to the bash executable or just "bash" if it is in the PATH
+     * @return A bash command that runs the given inputs, e.g. ["bash", "-c", "grep -rl 0x1234 out/build-info"]
+     */
+    function toBashCommand(string[] memory inputs, string memory bashPath) internal pure returns (string[] memory) {
+        string memory commandString;
+        for (uint i = 0; i < inputs.length; i++) {
+            commandString = string.concat(commandString, inputs[i]);
+            if (i != inputs.length - 1) {
+                commandString = string.concat(commandString, " ");
+            }
+        }
+
+        string[] memory result = new string[](3);
+        result[0] = bashPath;
+        result[1] = "-c";
+        result[2] = commandString;
+        return result;
+    }
+
+    /**
+     * @dev Runs an arbitrary command using bash.
+     * @param inputs Inputs for a command, e.g. ["grep", "-rl", "0x1234", "out/build-info"]
+     * @return The result of the corresponding bash command as a Vm.FfiResult struct
+     */
+    function runAsBashCommand(string[] memory inputs) internal returns (Vm.FfiResult memory) {
+        Vm vm = Vm(CHEATCODE_ADDRESS);
+        string memory defaultBashPath = "bash";
+        string memory bashPath = vm.envOr("OPENZEPPELIN_BASH_PATH", defaultBashPath);
+
+        string[] memory bashCommand = toBashCommand(inputs, bashPath);
+        Vm.FfiResult memory result = vm.tryFfi(bashCommand);
+        if (result.exitCode != 0 && result.stdout.length == 0 && result.stderr.length == 0) {
+            // On Windows, using the bash executable from WSL leads to a non-zero exit code and no output
+            revert(
+                string.concat(
+                    'Failed to run bash command with "',
+                    bashCommand[0],
+                    '". If you are using Windows, set the OPENZEPPELIN_BASH_PATH environment variable to the fully qualified path of the bash executable. For example, if you are using Git for Windows, add the following line in the .env file of your project (using forward slashes):\nOPENZEPPELIN_BASH_PATH="C:/Program Files/Git/bin/bash"'
+                )
+            );
+        } else {
+            return result;
+        }
+    }
 }

test/Utils.t.sol

@@ -2,7 +2,7 @@
 pragma solidity ^0.8.20;
 
 import {Test} from "forge-std/Test.sol";
-import {strings} from "solidity-stringutils/strings.sol";
+import {strings} from "solidity-stringutils/src/strings.sol";
 
 import {Utils, ContractInfo} from "openzeppelin-foundry-upgrades/internal/Utils.sol";
 
@@ -107,6 +107,20 @@ contract UpgradesTest is Test {
         assertTrue(buildInfoFile.toSlice().startsWith("out/build-info".toSlice()));
         assertTrue(buildInfoFile.toSlice().endsWith(".json".toSlice()));
     }
+
+    function testToBashCommand() public {
+        string[] memory inputs = new string[](3);
+        inputs[0] = "foo";
+        inputs[1] = "param";
+        inputs[2] = "--option";
+
+        string[] memory bashCommand = Utils.toBashCommand(inputs, "bash");
+
+        assertEq(bashCommand.length, 3);
+        assertEq(bashCommand[0], "bash");
+        assertEq(bashCommand[1], "-c");
+        assertEq(bashCommand[2], "foo param --option");
+    }
 }
 
 contract Invoker {