Fix #2696: Segfault with bad dub.sdl

source/dub/project.d

@@ -491,7 +491,9 @@ class Project {
 		m_missingDependencies = [];
 
 		Package resolveSubPackage(Package p, string subname, bool silentFail) {
-			return subname.length ? m_packageManager.getSubPackage(p, subname, silentFail) : p;
+			if (!subname.length || p is null)
+				return p;
+			return m_packageManager.getSubPackage(p, subname, silentFail);
 		}
 
 		void collectDependenciesRec(Package pack, int depth = 0)
@@ -556,6 +558,9 @@ class Project {
 					if (!vspec.repository.empty) {
 						p = m_packageManager.loadSCMPackage(basename, vspec.repository);
 						resolveSubPackage(p, subname, false);
+						enforce(p !is null,
+							"Unable to fetch '%s@%s' using git - does the repository and version exists?".format(
+								dep.name, vspec.repository));
 					} else if (!vspec.path.empty && is_desired) {
 						NativePath path = vspec.path;
 						if (!path.absolute) path = pack.path ~ path;

source/dub/test/base.d

@@ -168,6 +168,9 @@ public class TestSelectedVersions : SelectedVersions {
  */
 package class TestPackageManager : PackageManager
 {
+    /// List of all SCM packages that can be fetched by this instance
+    protected Package[Repository] scm;
+
     this()
     {
         NativePath pkg = NativePath("/tmp/dub-testsuite-nonexistant/packages/");
@@ -224,6 +227,43 @@ package class TestPackageManager : PackageManager
 		return null;
     }
 
+	/**
+	 * Re-Implementation of `loadSCMPackage`.
+	 *
+	 * The base implementation will do a `git` clone, which we would like to avoid.
+	 * Instead, we allow unittests to explicitly define what packages should be
+	 * reachable in a given test.
+	 */
+	public override Package loadSCMPackage(string name, Repository repo)
+	{
+        import std.string : chompPrefix;
+
+		// We're trying to match `loadGitPackage` as much as possible
+		if (!repo.ref_.startsWith("~") && !repo.ref_.isGitHash)
+			return null;
+
+		string gitReference = repo.ref_.chompPrefix("~");
+		NativePath destination = this.getPackagePath(PlacementLocation.user, name, repo.ref_);
+		destination ~= name;
+		destination.endsWithSlash = true;
+
+		foreach (p; getPackageIterator(name))
+			if (p.path == destination)
+				return p;
+
+		return this.loadSCMRepository(name, repo);
+	}
+
+	/// The private part of `loadSCMPackage`
+	protected Package loadSCMRepository(string name, Repository repo)
+	{
+		if (auto prepo = repo in this.scm) {
+            this.add(*prepo);
+            return *prepo;
+        }
+		return null;
+	}
+
     /**
      * Adds a `Package` to this `PackageManager`
      *
@@ -238,6 +278,12 @@ package class TestPackageManager : PackageManager
 		this.m_internal.fromPath ~= pkg;
 		return pkg;
 	}
+
+    /// Add a reachable SCM package to this `PackageManager`
+    public void addTestSCMPackage(Repository repo, Package pkg)
+    {
+        this.scm[repo] = pkg;
+    }
 }
 
 /**

source/dub/test/other.d

@@ -0,0 +1,50 @@
+/*******************************************************************************
+
+    Tests that don't fit in existing categories
+
+*******************************************************************************/
+
+module dub.test.others;
+
+version (unittest):
+
+import std.algorithm;
+import std.format;
+import dub.test.base;
+
+// https://github.com/dlang/dub/issues/2696
+unittest
+{
+    const ValidURL = `git+https://example.com/dlang/dub`;
+    // Taken from a commit in the dub repository
+    const ValidHash = "54339dff7ce9ec24eda550f8055354f712f15800";
+    const Template = `{"name": "%s", "dependencies": {
+"dep1": { "repository": "%s", "version": "%s" }}}`;
+
+    scope dub = new TestDub();
+    dub.packageManager.addTestSCMPackage(
+        Repository(ValidURL, ValidHash),
+        // Note: SCM package are always marked as using `~master`
+        dub.makeTestPackage(`{ "name": "dep1" }`, Version(`~master`)),
+    );
+
+    // Invalid URL, valid hash
+    const a = Template.format("a", "git+https://nope.nope", ValidHash);
+    try
+        dub.loadPackage(dub.addTestPackage(a, Version("1.0.0")));
+    catch (Exception exc)
+        assert(exc.message.canFind("Unable to fetch"));
+
+    // Valid URL, invalid hash
+    const b = Template.format("b", ValidURL, "invalid");
+    try
+        dub.loadPackage(dub.addTestPackage(b, Version("1.0.0")));
+    catch (Exception exc)
+        assert(exc.message.canFind("Unable to fetch"));
+
+    // Valid URL, valid hash
+    const c = Template.format("c", ValidURL, ValidHash);
+    dub.loadPackage(dub.addTestPackage(c, Version("1.0.0")));
+    assert(dub.project.hasAllDependencies());
+    assert(dub.project.getDependency("dep1", true), "Missing 'dep1' dependency");
+}