From 7f7bedc4653d02a18f5c4c972d4d034ab9e2c46f Mon Sep 17 00:00:00 2001 From: Nick Sabalausky Date: Sun, 26 Jul 2015 17:23:34 -0400 Subject: [PATCH] Fix #616: Infinite new-process recursion using dub decribe within preGenerateCommands --- source/dub/generators/build.d | 6 +- source/dub/generators/generator.d | 28 ++++++---- source/dub/package_.d | 55 +++++++++++++++++++ .../issue616-describe-vs-generate-commands.sh | 38 +++++++++++++ .../.no_build | 1 + .../do-preGenerateCommands.sh | 10 ++++ .../dub.json | 11 ++++ .../src/dummy.d | 1 + test/issue616-subpack/.no_build | 1 + test/issue616-subpack/dub.json | 10 ++++ test/issue616-subpack/src/dummy.d | 1 + test/issue616-subsubpack/.no_build | 1 + test/issue616-subsubpack/dub.json | 4 ++ test/issue616-subsubpack/src/dummy.d | 1 + 14 files changed, 155 insertions(+), 13 deletions(-) create mode 100755 test/issue616-describe-vs-generate-commands.sh create mode 100644 test/issue616-describe-vs-generate-commands/.no_build create mode 100755 test/issue616-describe-vs-generate-commands/do-preGenerateCommands.sh create mode 100644 test/issue616-describe-vs-generate-commands/dub.json create mode 100644 test/issue616-describe-vs-generate-commands/src/dummy.d create mode 100644 test/issue616-subpack/.no_build create mode 100644 test/issue616-subpack/dub.json create mode 100644 test/issue616-subpack/src/dummy.d create mode 100644 test/issue616-subsubpack/.no_build create mode 100644 test/issue616-subsubpack/dub.json create mode 100644 test/issue616-subsubpack/src/dummy.d diff --git a/source/dub/generators/build.d b/source/dub/generators/build.d index 09816b1c0..56a69505c 100644 --- a/source/dub/generators/build.d +++ b/source/dub/generators/build.d @@ -118,7 +118,7 @@ class BuildGenerator : ProjectGenerator { // run post-build commands if (!cached && buildsettings.postBuildCommands.length) { logInfo("Running post-build commands..."); - runBuildCommands(buildsettings.postBuildCommands, pack, settings, buildsettings); + runBuildCommands(buildsettings.postBuildCommands, pack, m_project, settings, buildsettings); } } @@ -148,7 +148,7 @@ class BuildGenerator : ProjectGenerator { if( buildsettings.preBuildCommands.length ){ logInfo("Running pre-build commands..."); - runBuildCommands(buildsettings.preBuildCommands, pack, settings, buildsettings); + runBuildCommands(buildsettings.preBuildCommands, pack, m_project, settings, buildsettings); } // override target path @@ -270,7 +270,7 @@ class BuildGenerator : ProjectGenerator { if( buildsettings.preBuildCommands.length ){ logInfo("Running pre-build commands..."); - runBuildCommands(buildsettings.preBuildCommands, pack, settings, buildsettings); + runBuildCommands(buildsettings.preBuildCommands, pack, m_project, settings, buildsettings); } buildWithCompiler(settings, buildsettings); diff --git a/source/dub/generators/generator.d b/source/dub/generators/generator.d index 1d53b4e50..5cdfc0432 100644 --- a/source/dub/generators/generator.d +++ b/source/dub/generators/generator.d @@ -93,7 +93,7 @@ class ProjectGenerator foreach (pack; m_project.getTopologicalPackageList(true, null, configs)) { BuildSettings buildsettings; buildsettings.processVars(m_project, pack, pack.getBuildSettings(settings.platform, configs[pack.name]), true); - prepareGeneration(pack, settings, buildsettings); + prepareGeneration(pack, m_project, settings, buildsettings); } string[] mainfiles; @@ -109,7 +109,7 @@ class ProjectGenerator BuildSettings buildsettings; buildsettings.processVars(m_project, pack, pack.getBuildSettings(settings.platform, configs[pack.name]), true); bool generate_binary = !(buildsettings.options & BuildOption.syntaxOnly); - finalizeGeneration(pack, settings, buildsettings, Path(bs.targetPath), generate_binary); + finalizeGeneration(pack, m_project, settings, buildsettings, Path(bs.targetPath), generate_binary); } performPostGenerateActions(settings, targets); @@ -332,23 +332,24 @@ ProjectGenerator createProjectGenerator(string generator_type, Project project) /** Runs pre-build commands and performs other required setup before project files are generated. */ -private void prepareGeneration(in Package pack, in GeneratorSettings settings, in BuildSettings buildsettings) +private void prepareGeneration(in Package pack, in Project proj, in GeneratorSettings settings, + in BuildSettings buildsettings) { - if (buildsettings.preGenerateCommands.length) { + if (buildsettings.preGenerateCommands.length && !PackagesUsed.has(pack.name)) { logInfo("Running pre-generate commands for %s...", pack.name); - runBuildCommands(buildsettings.preGenerateCommands, pack, settings, buildsettings); + runBuildCommands(buildsettings.preGenerateCommands, pack, proj, settings, buildsettings); } } /** Runs post-build commands and copies required files to the binary directory. */ -private void finalizeGeneration(in Package pack, in GeneratorSettings settings, in BuildSettings buildsettings, - Path target_path, bool generate_binary) +private void finalizeGeneration(in Package pack, in Project proj, in GeneratorSettings settings, + in BuildSettings buildsettings, Path target_path, bool generate_binary) { - if (buildsettings.postGenerateCommands.length) { + if (buildsettings.postGenerateCommands.length && !PackagesUsed.has(pack.name)) { logInfo("Running post-generate commands for %s...", pack.name); - runBuildCommands(buildsettings.postGenerateCommands, pack, settings, buildsettings); + runBuildCommands(buildsettings.postGenerateCommands, pack, proj, settings, buildsettings); } if (generate_binary) { @@ -440,7 +441,8 @@ private void finalizeGeneration(in Package pack, in GeneratorSettings settings, } } -void runBuildCommands(in string[] commands, in Package pack, in GeneratorSettings settings, in BuildSettings build_settings) +void runBuildCommands(in string[] commands, in Package pack, in Project proj, + in GeneratorSettings settings, in BuildSettings build_settings) { import std.conv; import std.process; @@ -485,5 +487,11 @@ void runBuildCommands(in string[] commands, in Package pack, in GeneratorSetting env["DUB_PARALLEL_BUILD"] = settings.parallelBuild? "TRUE" : ""; env["DUB_RUN_ARGS"] = (cast(string[])settings.runArgs).map!(escapeShellFileName).join(" "); + + PackagesUsed.add(proj.rootPackage.name); + foreach (dep; proj.dependencies) + PackagesUsed.add(dep.name); + PackagesUsed.store(env); + runCommands(commands, env); } diff --git a/source/dub/package_.d b/source/dub/package_.d index 2f06e151d..a7de09aab 100644 --- a/source/dub/package_.d +++ b/source/dub/package_.d @@ -631,3 +631,58 @@ private string determineVersionFromSCM(Path path) return null; } + +/// All packages being used by all parent DUB processes (if any). +/// Used to avoid infinite process recursion. +static class PackagesUsed +{ + import std.process : environment; +import std.stdio; + + private static bool loaded = false; + private static bool[string] packageNames; + private enum envVarName = "DUB_PACKAGES_USED"; + + static void add(string pack) + { + ensureLoaded(); + packageNames[pack] = true; + } + + static void add(string[] packs) + { + ensureLoaded(); + foreach (p; packs) + packageNames[p] = true; + } + + static bool has(string pack) + { + ensureLoaded(); + return !!(pack in packageNames); + } + + // Store in envvar + static void store(string[string] env) + { + auto commaList = packageNames.keys.sort().join(","); + env[envVarName] = commaList; + } + + // Load from envvar + private static void load() + { + auto commaList = environment.get(envVarName); + loaded = true; + + foreach (name; commaList.split(",")) + add(name); + } + + // Load if not already loaded + private static void ensureLoaded() + { + if (!loaded) + load(); + } +} diff --git a/test/issue616-describe-vs-generate-commands.sh b/test/issue616-describe-vs-generate-commands.sh new file mode 100755 index 000000000..e4c28e869 --- /dev/null +++ b/test/issue616-describe-vs-generate-commands.sh @@ -0,0 +1,38 @@ +#!/bin/bash +set -e -o pipefail + +cd "$CURR_DIR"/issue616-describe-vs-generate-commands + +temp_file=`mktemp` +temp_file2=`mktemp` + +function cleanup { + rm $temp_file + rm $temp_file2 +} + +trap cleanup EXIT + +if ! $DUB describe --data-list --data=target-name \ + > "$temp_file" 2>&1; then + die 'Printing project data failed!' +fi + +# Create the expected output file to compare stdout against. +expected_file="$CURR_DIR/expected-issue616-output" +echo "preGenerateCommands: DUB_PACKAGES_USED=issue616-describe-vs-generate-commands,issue616-subpack,issue616-subsubpack" > "$expected_file" +echo "$CURR_DIR/issue616-describe-vs-generate-commands/src/" >> "$expected_file" +echo "$CURR_DIR/issue616-subpack/src/" >> "$expected_file" +echo "$CURR_DIR/issue616-subsubpack/src/" >> "$expected_file" +echo "issue616-describe-vs-generate-commands" >> "$expected_file" + +# Create the expected output file to compare stderr against. +expected_file2="$CURR_DIR/expected-issue616-output2" +echo "preGenerateCommands: DUB_PACKAGES_USED=issue616-describe-vs-generate-commands,issue616-subpack,issue616-subsubpack" > "$expected_file2" +echo "$CURR_DIR/issue616-describe-vs-generate-commands/src/" >> "$expected_file2" +echo "$CURR_DIR/issue616-subpack/src/" >> "$expected_file2" +echo "$CURR_DIR/issue616-subsubpack/src/" >> "$expected_file2" + +if ! diff "$expected_file" "$temp_file"; then + die 'The stdout output did not match the expected output!' +fi diff --git a/test/issue616-describe-vs-generate-commands/.no_build b/test/issue616-describe-vs-generate-commands/.no_build new file mode 100644 index 000000000..8d1c8b69c --- /dev/null +++ b/test/issue616-describe-vs-generate-commands/.no_build @@ -0,0 +1 @@ + diff --git a/test/issue616-describe-vs-generate-commands/do-preGenerateCommands.sh b/test/issue616-describe-vs-generate-commands/do-preGenerateCommands.sh new file mode 100755 index 000000000..7aa469a27 --- /dev/null +++ b/test/issue616-describe-vs-generate-commands/do-preGenerateCommands.sh @@ -0,0 +1,10 @@ +#!/bin/sh +if [ -n "${dub_issue616}" ]; then + echo 'Fail! preGenerateCommands recursion detected!' >&2 + exit 0 # Don't return a non-zero error code here. This way the test gives a better diagnostic. +fi + +echo preGenerateCommands: DUB_PACKAGES_USED=$DUB_PACKAGES_USED >&2 + +export dub_issue616=true +$DUB describe --data-list --data=import-paths >&2 diff --git a/test/issue616-describe-vs-generate-commands/dub.json b/test/issue616-describe-vs-generate-commands/dub.json new file mode 100644 index 000000000..8aec1eda0 --- /dev/null +++ b/test/issue616-describe-vs-generate-commands/dub.json @@ -0,0 +1,11 @@ +{ + "name": "issue616-describe-vs-generate-commands", + "targetType": "executable", + "preGenerateCommands-posix": ["cd $PACKAGE_DIR && ./do-preGenerateCommands.sh"], + "dependencies": { + "issue616-subpack": { + "version": "1.0", + "path": "../issue616-subpack" + } + } +} diff --git a/test/issue616-describe-vs-generate-commands/src/dummy.d b/test/issue616-describe-vs-generate-commands/src/dummy.d new file mode 100644 index 000000000..8d1c8b69c --- /dev/null +++ b/test/issue616-describe-vs-generate-commands/src/dummy.d @@ -0,0 +1 @@ + diff --git a/test/issue616-subpack/.no_build b/test/issue616-subpack/.no_build new file mode 100644 index 000000000..8d1c8b69c --- /dev/null +++ b/test/issue616-subpack/.no_build @@ -0,0 +1 @@ + diff --git a/test/issue616-subpack/dub.json b/test/issue616-subpack/dub.json new file mode 100644 index 000000000..552ddcdcd --- /dev/null +++ b/test/issue616-subpack/dub.json @@ -0,0 +1,10 @@ +{ + "name": "issue616-subpack", + "targetType": "executable", + "dependencies": { + "issue616-subsubpack": { + "version": "1.0", + "path": "../issue616-subsubpack" + } + } +} diff --git a/test/issue616-subpack/src/dummy.d b/test/issue616-subpack/src/dummy.d new file mode 100644 index 000000000..8d1c8b69c --- /dev/null +++ b/test/issue616-subpack/src/dummy.d @@ -0,0 +1 @@ + diff --git a/test/issue616-subsubpack/.no_build b/test/issue616-subsubpack/.no_build new file mode 100644 index 000000000..8d1c8b69c --- /dev/null +++ b/test/issue616-subsubpack/.no_build @@ -0,0 +1 @@ + diff --git a/test/issue616-subsubpack/dub.json b/test/issue616-subsubpack/dub.json new file mode 100644 index 000000000..e4e4b5b0d --- /dev/null +++ b/test/issue616-subsubpack/dub.json @@ -0,0 +1,4 @@ +{ + "name": "issue616-subsubpack", + "targetType": "executable" +} diff --git a/test/issue616-subsubpack/src/dummy.d b/test/issue616-subsubpack/src/dummy.d new file mode 100644 index 000000000..8d1c8b69c --- /dev/null +++ b/test/issue616-subsubpack/src/dummy.d @@ -0,0 +1 @@ +