Merge pull request #633 from Abscissa/fix616

Fix #616: Infinite new-process recursion using dub decribe within…

.gitignore

@@ -22,5 +22,6 @@ __dummy.html
 /test/expected-string-import-path-output
 /test/expected-describe-data-1-list-output
 /test/expected-describe-data-2-dmd-output
+/test/expected-issue616-output
 /test/describe-project/dummy.dat
 /test/describe-project/dummy-dep1.dat

source/dub/generators/build.d

@@ -127,7 +127,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);
 		}
 
 		return cached;
@@ -159,7 +159,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
@@ -281,7 +281,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);

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);
@@ -334,23 +334,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 && !isRecursiveInvocation(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 && !isRecursiveInvocation(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) {
@@ -442,7 +443,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;
@@ -487,5 +489,8 @@ 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(" ");
+
+	auto depNames = proj.dependencies.map!((a) => a.name).array();
+	storeRecursiveInvokations(env, proj.rootPackage.name ~ depNames);
 	runCommands(commands, env);
 }

source/dub/package_.d

@@ -632,3 +632,24 @@ private string determineVersionFromSCM(Path path)
 
 	return null;
 }
+
+bool isRecursiveInvocation(string pack)
+{
+	import std.process : environment;
+
+	return environment
+        .get("DUB_PACKAGES_USED", "")
+        .splitter(",")
+        .canFind(pack);
+}
+
+void storeRecursiveInvokations(string[string] env, string[] packs)
+{
+	import std.process : environment;
+
+    env["DUB_PACKAGES_USED"] = environment
+        .get("DUB_PACKAGES_USED", "")
+        .splitter(",")
+        .chain(packs)
+        .join(",");
+}

test/issue616-describe-vs-generate-commands.sh

@@ -0,0 +1,29 @@
+#!/bin/bash
+set -e -o pipefail
+
+cd "$CURR_DIR"/issue616-describe-vs-generate-commands
+
+temp_file=`mktemp`
+
+function cleanup {
+    rm $temp_file
+}
+
+trap cleanup EXIT
+
+if ! $DUB describe --compiler=$COMPILER --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"
+
+if ! diff "$expected_file" "$temp_file"; then
+    die 'The stdout output did not match the expected output!'
+fi

test/issue616-describe-vs-generate-commands/.no_build

@@ -0,0 +1 @@
+

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 --compiler=$COMPILER --data-list --data=import-paths >&2

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"
+        }
+    }
+}

test/issue616-describe-vs-generate-commands/src/dummy.d

@@ -0,0 +1 @@
+

test/issue616-subpack/.no_build

@@ -0,0 +1 @@
+

test/issue616-subpack/dub.json

@@ -0,0 +1,10 @@
+{
+    "name": "issue616-subpack",
+    "targetType": "executable",
+    "dependencies": {
+        "issue616-subsubpack": {
+            "version": "1.0",
+            "path": "../issue616-subsubpack"
+        }
+    }
+}

test/issue616-subpack/src/dummy.d

@@ -0,0 +1 @@
+

test/issue616-subsubpack/.no_build

@@ -0,0 +1 @@
+

test/issue616-subsubpack/dub.json

@@ -0,0 +1,4 @@
+{
+    "name": "issue616-subsubpack",
+    "targetType": "executable"
+}

test/issue616-subsubpack/src/dummy.d

@@ -0,0 +1 @@
+