Skip to content

Commit

Permalink
Fix #616: Infinite new-process recursion using dub decribe within pre…
Browse files Browse the repository at this point in the history
…GenerateCommands
  • Loading branch information
Abscissa committed Jul 26, 2015
1 parent bc684aa commit 7f7bedc
Show file tree
Hide file tree
Showing 14 changed files with 155 additions and 13 deletions.
6 changes: 3 additions & 3 deletions source/dub/generators/build.d
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 18 additions & 10 deletions source/dub/generators/generator.d
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
55 changes: 55 additions & 0 deletions source/dub/package_.d
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
38 changes: 38 additions & 0 deletions test/issue616-describe-vs-generate-commands.sh
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions test/issue616-describe-vs-generate-commands/.no_build
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions test/issue616-describe-vs-generate-commands/dub.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
1 change: 1 addition & 0 deletions test/issue616-describe-vs-generate-commands/src/dummy.d
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions test/issue616-subpack/.no_build
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

10 changes: 10 additions & 0 deletions test/issue616-subpack/dub.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "issue616-subpack",
"targetType": "executable",
"dependencies": {
"issue616-subsubpack": {
"version": "1.0",
"path": "../issue616-subsubpack"
}
}
}
1 change: 1 addition & 0 deletions test/issue616-subpack/src/dummy.d
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions test/issue616-subsubpack/.no_build
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

4 changes: 4 additions & 0 deletions test/issue616-subsubpack/dub.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "issue616-subsubpack",
"targetType": "executable"
}
1 change: 1 addition & 0 deletions test/issue616-subsubpack/src/dummy.d
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

0 comments on commit 7f7bedc

Please sign in to comment.