cleanup trailing slash/backslash behavior of expanded vars

- expand $VARS without trailing slash/backslash (for concatenation e.g. `${PACKAGE_PATH}/subpath`)
dlang · Feb 23, 2018 · 73ec880 · 73ec880
1 parent 3e875ec
commit 73ec880
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,9 @@ Changelog
 v1.9.0 - 2018-05-01
 -------------------
 
-- support braced `${MY_VAR}` env var expansions, e.g. for `${PACKAGE_PATH}relative` - [issue #1392][issue1392]
+- support braced `${MY_VAR}` env var expansions, e.g. for `${PACKAGE_PATH}_suffix` - [issue #1392][issue1392]
+- `$PACKAGE_PATH`, `$ROOT_PACKAGE_PATH`, and `$DEP_PACKAGE_PATH` no longer end
+  with a `/` or `\` to support clean concatenation, e.g. `${PACKAGE_PATH}/subpath` - [issue #1392][issue1392]
 
 [issue1392]: https://github.com/dlang/dub/issues/1392
 

diff --git a/source/dub/project.d b/source/dub/project.d
@@ -1226,14 +1226,27 @@ private string processVars(Project, Package)(string var, in Project project, in
 private string getVariable(Project, Package)(string name, in Project project, in Package pack)
 {
 	import std.process : environment;
-	if (name == "PACKAGE_DIR") return pack.path.toNativeString();
-	if (name == "ROOT_PACKAGE_DIR") return project.rootPackage.path.toNativeString();
+	Path path;
+	if (name == "PACKAGE_DIR")
+		path = pack.path;
+	else if (name == "ROOT_PACKAGE_DIR")
+		path = project.rootPackage.path;
 
 	if (name.endsWith("_PACKAGE_DIR")) {
 		auto pname = name[0 .. $-12];
 		foreach (prj; project.getTopologicalPackageList())
 			if (prj.name.toUpper().replace("-", "_") == pname)
-				return prj.path.toNativeString();
+			{
+				path = prj.path;
+				break;
+			}
+	}
+
+	if (!path.empty)
+	{
+		// no trailing slash for clean path concatenation (see #1392)
+		path.endsWithSlash = false;
+		return path.toNativeString();
 	}
 
 	auto envvar = environment.get(name);
@@ -1251,9 +1264,11 @@ unittest
 		{
 			this.name = name;
 			version (Posix)
-				path = NativePath("/pkgs/"~name~"/");
+				path = NativePath("/pkgs/"~name);
 			else version (Windows)
-				path = NativePath(`C:\pkgs\`~name~`\`);
+				path = NativePath(`C:\pkgs\`~name);
+			// see 4d4017c14c, #268, and #1392 for why this all package paths end on slash internally
+			path.endsWithSlash = true;
 		}
 		string name;
 		NativePath path;
@@ -1276,12 +1291,17 @@ unittest
 	};
 	auto pack = MockPackage("test");
 	enum isPath = true;
+
+	version (Posix) enum sep = "/";
+	else version (Windows) enum sep = `\`;
+	static Path woSlash(Path p) { p.endsWithSlash = false; return p; }
 	// basic vars
-	assert(processVars("Hello $PACKAGE_DIR", proj, pack, !isPath) == "Hello "~pack.path.toNativeString);
-	assert(processVars("Hello $ROOT_PACKAGE_DIR", proj, pack, !isPath) == "Hello "~proj.rootPackage.path.toNativeString);
-	assert(processVars("Hello $DEP1_PACKAGE_DIR", proj, pack, !isPath) == "Hello "~proj._dependencies[0].path.toNativeString);
-	// ${VAR} replacements, NOTE: PACKAGE_DIR et.al. end on /
-	assert(processVars("Hello ${PACKAGE_DIR}foobar", proj, pack, !isPath) == "Hello "~(pack.path ~ "foobar").toNativeString);
+	assert(processVars("Hello $PACKAGE_DIR", proj, pack, !isPath) == "Hello "~woSlash(pack.path).toNativeString);
+	assert(processVars("Hello $ROOT_PACKAGE_DIR", proj, pack, !isPath) == "Hello "~woSlash(proj.rootPackage.path).toNativeString.chomp(sep));
+	assert(processVars("Hello $DEP1_PACKAGE_DIR", proj, pack, !isPath) == "Hello "~woSlash(proj._dependencies[0].path).toNativeString);
+	// ${VAR} replacements
+	assert(processVars("Hello ${PACKAGE_DIR}"~sep~"foobar", proj, pack, !isPath) == "Hello "~(pack.path ~ "foobar").toNativeString);
+	assert(processVars("Hello $PACKAGE_DIR"~sep~"foobar", proj, pack, !isPath) == "Hello "~(pack.path ~ "foobar").toNativeString);
 	// test with isPath
 	assert(processVars("local", proj, pack, isPath) == (pack.path ~ "local").toNativeString);
 	// test other env variables