Fixed problems with copying the data folder for nested remote filters…

…, added test for this case. Close #32

regolith/project.go

@@ -90,8 +90,10 @@ func LoadFiltersFromPath(path string) (*Profile, error) {
 // LoadFilterJsonProfile loads a profile from path to filter.json file of
 // a remote filter and propagates the properties of the parent filter (the
 // filter in config.json or other remote filter that caused creation of this
-// profile).
-func LoadFilterJsonProfile(filterJsonPath string, parentFilter Filter) (*Profile, error) {
+// profile).and the parent profile to the returned profile.
+func LoadFilterJsonProfile(
+	filterJsonPath string, parentFilter Filter, parentProfile Profile,
+) (*Profile, error) {
 	// Open file
 	file, err := ioutil.ReadFile(filterJsonPath)
 	if err != nil {
@@ -109,6 +111,9 @@ func LoadFilterJsonProfile(filterJsonPath string, parentFilter Filter) (*Profile
 	for subfilter := range remoteProfile.Filters {
 		remoteProfile.Filters[subfilter].VenvSlot = parentFilter.VenvSlot
 	}
+	remoteProfile.DataPath = parentProfile.DataPath
+	remoteProfile.ExportTarget = parentProfile.ExportTarget
+	remoteProfile.Unsafe = parentProfile.Unsafe
 	return &remoteProfile, nil
 }
 
@@ -164,7 +169,7 @@ func (p *Profile) Install(isForced bool, profilePath string) error {
 
 		// Create profile from filter.json file
 		remoteProfile, err := LoadFilterJsonProfile(
-			filepath.Join(downloadPath, "filter.json"), *filter)
+			filepath.Join(downloadPath, "filter.json"), *filter, *p)
 		if err != nil {
 			return err // TODO - I don't think this should break the entire install. Just remove the files and continue.
 		}

test/common.go

@@ -0,0 +1,102 @@
+package test
+
+import (
+	"crypto/md5"
+	"encoding/hex"
+	"io/fs"
+	"io/ioutil"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+const freshProjectPath = "testdata/fresh_project"
+const minimalProjectPath = "testdata/minimal_project"
+const multitargetProjectPath = "testdata/multitarget_project"
+const doubleRemoteProjectPath = "testdata/double_remote_project"
+const doubleRemoteProjectInstalledPath = "testdata/double_remote_project_installed"
+
+// listPaths returns a dictionary with paths of the files from 'path' directory
+// relative to 'root' directory used as keys, and with md5 hashes paths as
+// values. The directory paths use empty strings instead of MD5. The function
+// ignores files called .ignoreme (they simulate empty directories
+// in git repository).
+func listPaths(path string, root string) (map[string]string, error) {
+	result := map[string]string{}
+	err := filepath.WalkDir(path,
+		func(path string, data fs.DirEntry, err error) error {
+			if err != nil {
+				return err
+			}
+			if data.Name() == ".ignoreme" { // Ignored file
+				return nil
+			}
+			relPath, err := filepath.Rel(root, path)
+			if err != nil {
+				return err
+			}
+			if data.IsDir() {
+				result[relPath] = ""
+			} else {
+				content, err := ioutil.ReadFile(path)
+				if err != nil {
+					return err
+				}
+				hash := md5.New()
+				// Get the hash value, ignore carriage return
+				hash.Write([]byte(strings.Replace(string(content), "\r", "", -1)))
+				hashInBytes := hash.Sum(nil)
+				result[relPath] = hex.EncodeToString(hashInBytes)
+			}
+			return nil
+		})
+	if err != nil {
+		return map[string]string{}, err
+	}
+	return result, nil
+}
+
+// comparePathMaps compares maps created by listPaths function and runs
+// t.Fatal in case of finding a difference.
+func comparePathMaps(
+	expectedPaths map[string]string, createdPaths map[string]string,
+	t *testing.T,
+) {
+	checked := struct{}{}
+	checklist := map[string]struct{}{}
+	// Check if all expectedPaths are created
+	for k, expectedHash := range expectedPaths {
+		checklist[k] = checked
+		createdHash, exists := createdPaths[k]
+		if !exists {
+			t.Fatal("Missing expected path:", k)
+		} else if createdHash != expectedHash {
+			if expectedHash == "" {
+				t.Fatalf("%q should be a file but is a directory instead", k)
+			} else if createdHash == "" {
+				t.Fatalf("%q should be a directory but is a file instead", k)
+			}
+			// Print the file, that doesn't match
+			//bytes, _ := ioutil.ReadFile(k)
+			//t.Log(string(bytes))
+			t.Fatalf("%q file is different that expected", k)
+		}
+	}
+	// Check if all createdPaths are expected
+	for k, createdHash := range createdPaths {
+		if _, checked := checklist[k]; checked {
+			continue // This is checked already (skip)
+		}
+		expectedHash, exists := expectedPaths[k]
+		if !exists {
+			t.Fatal("Additional unexpected path was created:", k)
+		} else if createdHash != expectedHash {
+			if expectedHash == "" {
+				t.Fatalf("%q should be a file but is a directory instead", k)
+			} else if createdHash == "" {
+				t.Fatalf("%q should be a directory but is a file instead", k)
+			}
+			t.Fatalf("%q file is different that expected", k)
+		}
+	}
+}

test/project_test.go

@@ -1,58 +1,13 @@
 package test
 
 import (
-	"crypto/md5"
-	"encoding/hex"
-	"io/fs"
 	"io/ioutil"
 	"os"
-	"path/filepath"
-	"strings"
 	"testing"
 
 	"bedrock-oss.github.com/regolith/regolith"
 )
 
-// listPaths returns a dictionary with paths of the files from 'path' directory
-// relative to 'root' directory used as keys, and with md5 hashes paths as
-// values. The directory paths use empty strings instead of MD5. The function
-// ignores files called .ignoreme (they simulate empty directories
-// in git repository).
-func listPaths(path string, root string) (map[string]string, error) {
-	result := map[string]string{}
-	err := filepath.WalkDir(path,
-		func(path string, data fs.DirEntry, err error) error {
-			if err != nil {
-				return err
-			}
-			if data.Name() == ".ignoreme" { // Ignored file
-				return nil
-			}
-			relPath, err := filepath.Rel(root, path)
-			if err != nil {
-				return err
-			}
-			if data.IsDir() {
-				result[relPath] = ""
-			} else {
-				content, err := ioutil.ReadFile(path)
-				if err != nil {
-					return err
-				}
-				hash := md5.New()
-				// Get the hash value, ignore carriage return
-				hash.Write([]byte(strings.Replace(string(content), "\r", "", -1)))
-				hashInBytes := hash.Sum(nil)
-				result[relPath] = hex.EncodeToString(hashInBytes)
-			}
-			return nil
-		})
-	if err != nil {
-		return map[string]string{}, err
-	}
-	return result, nil
-}
-
 // TestRegolithInit tests the results of InitializeRegolithProject against
 // the values from test/testdata/fresh_project.
 func TestRegolithInit(t *testing.T) {
@@ -89,42 +44,5 @@ func TestRegolithInit(t *testing.T) {
 	if err != nil {
 		t.Fatal("Unable to get list of created paths:", err)
 	}
-
-	checked := struct{}{}
-	checklist := map[string]struct{}{}
-	// Check if all expectedPaths are created
-	for k, expectedHash := range expectedPaths {
-		checklist[k] = checked
-		createdHash, exists := createdPaths[k]
-		if !exists {
-			t.Fatal("Missing expected path:", k)
-		} else if createdHash != expectedHash {
-			if expectedHash == "" {
-				t.Fatalf("%q should be a file but is a directory instead", k)
-			} else if createdHash == "" {
-				t.Fatalf("%q should be a directory but is a file instead", k)
-			}
-			// Print the file, that doesn't match
-			//bytes, _ := ioutil.ReadFile(k)
-			//t.Log(string(bytes))
-			t.Fatalf("%q file is different that expected", k)
-		}
-	}
-	// Check if all createdPaths are expected
-	for k, createdHash := range createdPaths {
-		if _, checked := checklist[k]; checked {
-			continue // This is checked already (skip)
-		}
-		expectedHash, exists := expectedPaths[k]
-		if !exists {
-			t.Fatal("Additional unexpected path was created:", k)
-		} else if createdHash != expectedHash {
-			if expectedHash == "" {
-				t.Fatalf("%q should be a file but is a directory instead", k)
-			} else if createdHash == "" {
-				t.Fatalf("%q should be a directory but is a file instead", k)
-			}
-			t.Fatalf("%q file is different that expected", k)
-		}
-	}
+	comparePathMaps(expectedPaths, createdPaths, t)
 }

test/remote_filters_test.go

@@ -0,0 +1,64 @@
+package test
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"bedrock-oss.github.com/regolith/regolith"
+	"github.com/otiai10/copy"
+)
+
+// TestDoubleRemoteFilter tests how regolith handles installing a remote
+// with dependencies that point at another remote filter.
+func TestDoubleRemoteFilter(t *testing.T) {
+	// Switching working directories in this test, make sure to go back
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatal("Unable to get current working directory")
+	}
+	defer os.Chdir(wd)
+	// Load expected output
+	expectedPaths, err := listPaths(
+		doubleRemoteProjectInstalledPath, doubleRemoteProjectInstalledPath)
+	if err != nil {
+		t.Fatal("Unable load the expected paths:", err)
+	}
+	// Create a temporary directory
+	tmpDir, err := ioutil.TempDir("", "regolith-test")
+	if err != nil {
+		t.Fatal("Unable to create temporary directory:", err)
+	}
+	t.Log("Created temporary directory:", tmpDir)
+	// Before deleting "workingDir" the test must stop using it
+	defer os.RemoveAll(tmpDir)
+	defer os.Chdir(wd)
+	workingDir := filepath.Join(tmpDir, "working-dir")
+	os.Mkdir(workingDir, 0666)
+	// Copy the test project to the working directory
+	err = copy.Copy(
+		doubleRemoteProjectPath,
+		workingDir,
+		copy.Options{PreserveTimes: false, Sync: false},
+	)
+	if err != nil {
+		t.Fatalf(
+			"Failed to copy test files %q into the working directory %q",
+			doubleRemoteProjectPath, workingDir,
+		)
+	}
+	// Switch to the working directory
+	os.Chdir(workingDir)
+	// Run InstallDependencies
+	regolith.InitLogging(true)
+	regolith.RegisterFilters()
+	regolith.InstallDependencies(false)
+	// Load created paths for comparison with expected output
+	createdPaths, err := listPaths(".", ".")
+	if err != nil {
+		t.Fatal("Unable to load the created paths:", err)
+	}
+	// Compare the installed dependencies with the expected dependencies
+	comparePathMaps(expectedPaths, createdPaths, t)
+}

test/testdata/README.md

@@ -8,3 +8,8 @@ This folder is used to store resources used for testing.
     path.
 - `multitarget_project` - a copy of `minimal_project` but with modified
     config.json, to add multiple profiles with different export targets.
+- `double_remote_project` - a project that uses a remote filter from
+    [regolith-test-filters](https://github.com/Bedrock-OSS/regolith-test-filters).
+    The filter has a reference to another remote filter on the same reposiotry.
+- `double_remote_project_installed` - expected result of contents of
+    `double_remote_project` after installation.

test/testdata/double_remote_project/.gitignore

@@ -0,0 +1,2 @@
+/build
+/.regolith

test/testdata/double_remote_project/config.json

@@ -0,0 +1,24 @@
+{
+  "name": "regolith_test_project",
+  "author": "Bedrock-OSS",
+  "packs": {
+    "behaviorPack": "./packs/BP",
+    "resourcePack": "./packs/RP"
+  },
+  "regolith": {
+    "profiles": {
+      "dev": {
+        "unsafe": false,
+        "filters": [
+          {
+            "url": "github.com/Bedrock-OSS/regolith-test-filters/nested-remote-filter"
+          }
+        ],
+        "export": {
+          "target": "development"
+        },
+        "dataPath": "./packs/data"
+      }
+    }
+  }
+}

test/testdata/double_remote_project/packs/BP/manifest.json

@@ -0,0 +1,23 @@
+{
+    "format_version": 2,
+    "header": {
+        "description": "This is test BP",
+        "name": "Regolith Test BP",
+        "uuid": "96b53fd2-b7a1-4d26-b74f-1b9394c8d0bc",
+        "version": [1, 0, 0],
+        "min_engine_version": [1, 16, 0]
+    },
+    "modules": [
+        {
+            "type": "data",
+            "uuid": "4eef1f3f-91b5-43df-b5ab-07e9aa89081b",
+            "version": [1, 0, 0]
+        }
+    ],
+    "dependencies": [
+        {
+            "uuid": "6f6e3f0b-1627-488d-a9aa-2d1430ba368a",
+            "version": [1, 0, 0]
+        }
+    ]
+}

test/testdata/double_remote_project/packs/RP/manifest.json

@@ -0,0 +1,17 @@
+{
+    "format_version": 2,
+    "header": {
+        "description": "This is test RP",
+        "name": "Regolith Test RP",
+        "uuid": "6f6e3f0b-1627-488d-a9aa-2d1430ba368a",
+        "version": [1, 0, 0],
+        "min_engine_version": [1, 16, 0]
+    },
+    "modules": [
+        {
+            "type": "resources",
+            "uuid": "65b1ba69-462d-4199-aa3b-a0f161ed0bde",
+            "version": [1, 0, 0]
+        }
+    ]
+}

test/testdata/double_remote_project/packs/data/example_data_file.json

@@ -0,0 +1 @@
+{}

test/testdata/double_remote_project_installed/.gitignore

@@ -0,0 +1,2 @@
+/build
+/.regolith