diff --git a/regolith/filters.go b/regolith/filters.go index 34222519..af0f4e86 100644 --- a/regolith/filters.go +++ b/regolith/filters.go @@ -30,12 +30,12 @@ func RegisterFilters() { // absoluteLocation is an absolute path to the root folder of the filter. // In case of local filters it's a root path of the project. func (filter *Filter) RunFilter(absoluteLocation string) error { - Logger.Info(filter.GetName()) + Logger.Infof("Running filter %s", filter.GetFriendlyName()) start := time.Now() // Disabled filters are skipped if filter.Disabled == true { - Logger.Infof("Filter '%s' is disabled, skipping.", filter.GetName()) + Logger.Infof("Filter '%s' is disabled, skipping.", filter.GetFriendlyName()) return nil } diff --git a/regolith/profiles.go b/regolith/profiles.go index c893866c..62f80e8e 100644 --- a/regolith/profiles.go +++ b/regolith/profiles.go @@ -98,7 +98,7 @@ func RunProfile(profileName string) error { path, _ := filepath.Abs(".") err := filter.RunFilter(path) if err != nil { - return wrapError(fmt.Sprintf("%s failed", filter.GetName()), err) + return wrapError(fmt.Sprintf("%s failed", filter.GetFriendlyName()), err) } } diff --git a/regolith/project.go b/regolith/project.go index 5d48c5f1..7c874c0b 100644 --- a/regolith/project.go +++ b/regolith/project.go @@ -61,12 +61,13 @@ type ExportTarget struct { // Path string `json:"path"` } -// TODO This function is kinda weird. Probably should not include "Running filter" -func (filter Filter) GetName() string { +// GetFriendlyName returns the friendly name of the filter that can be used for +// logging. +func (filter Filter) GetFriendlyName() string { if filter.Name != "" { return filter.Name } - return fmt.Sprintf("Running filter %s", filter.Filter) + return filter.Filter } func IsProjectConfigured() bool { diff --git a/regolith/remote_filters.go b/regolith/remote_filters.go index b300488c..c2f97149 100644 --- a/regolith/remote_filters.go +++ b/regolith/remote_filters.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path" + "path/filepath" "strings" getter "github.com/hashicorp/go-getter" @@ -38,11 +39,9 @@ func IsRemoteFilterCached(url string) bool { return err == nil } -/* -Recursively install dependencies for the entire config. - - Force mode will overwrite existing dependencies. - - Non-force mode will only install dependencies that are not already installed. -*/ +// Recursively install dependencies for the entire config. +// - Force mode will overwrite existing dependencies. +// - Non-force mode will only install dependencies that are not already installed. func InstallDependencies(isForced bool) error { Logger.Infof("Installing dependencies...") @@ -63,7 +62,7 @@ func InstallDependencies(isForced bool) error { } for _, profile := range project.Profiles { - err := InstallDependency(profile, isForced) + err := profile.Install(isForced) if err != nil { return wrapError("Could not install dependency", err) } @@ -73,133 +72,193 @@ func InstallDependencies(isForced bool) error { return nil } -// InstallDependency recursively downloads the filters of a profile and the -// filters specified in other filters. -func InstallDependency(profile Profile, isForced bool) error { - for _, filter := range profile.Filters { - // Get the url of the dependency, which may be constructed - var url string - if filter.Url != "" { - url = FilterNameToUrl(filter.Url, filter.Filter) - } else if filter.Filter != "" { - url = FilterNameToUrl(StandardLibraryUrl, filter.Filter) - } else { // Leaf of profile tree (nothing to install) - continue - } - - // TODO This needs to be re-implemented - // err := ValidateUrl(url) - // if err != nil { - // return err - // } - - // Download the filter into the cache folder - downloadPath := UrlToPath(url) - - // If downloadPath already exists, we don't need to download it again. - // Force mode allows overwriting. - if _, err := os.Stat(downloadPath); err == nil { - if !isForced { - Logger.Warnf("Dependency %s already installed, skipping. Run with '-f' to force.", url) - continue - } else { - Logger.Warnf("Dependency %s already installed, but forcing installation.", url) - err := os.RemoveAll(downloadPath) - if err != nil { - return wrapError("Could not remove installed filter", err) - } - } - } - - Logger.Infof("Installing dependency %s...", url) +// 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) { + // Open file + file, err := ioutil.ReadFile(filterJsonPath) + if err != nil { + return nil, wrapError(fmt.Sprintf( + "Couldn't find %s", filterJsonPath), err) + } + // Load data into Profile struct + var remoteProfile Profile + err = json.Unmarshal(file, &remoteProfile) + if err != nil { + return nil, wrapError(fmt.Sprintf( + "Couldn't load %s: ", filterJsonPath), err) + } + // Propagate venvSlot property + for subfilter := range remoteProfile.Filters { + remoteProfile.Filters[subfilter].VenvSlot = parentFilter.VenvSlot + } + return &remoteProfile, nil +} - // Download the filter fresh - ok, err := DownloadGitHubUrl(url, downloadPath) +// Install installs all of the dependencies of the profile +func (p *Profile) Install(isForced bool) error { + for filter := range p.Filters { + filter := &p.Filters[filter] // Using pointer is faster than creating copies in the loop and gives more options + downloadPath, err := filter.Download(isForced) + // TODO - we could use type switch to handle different kinds of errors + // here. Download can fail on downloading or on cleaning the download + // path. It can also fail when isForced is false and the path already + // exists. if err != nil { - Logger.Debug(err) - } - if !ok { - Logger.Debug("Failed to download filter " + filter.Filter + " without git") - err := getter.Get(downloadPath, url) - if err != nil { - return err - } - } - - // Remove 'test' folder, which may be installed via git-getter library - // This is a workaround for cases where our own getter library is not - // able to download the filter. - testFolder := path.Join(downloadPath, "test") - if _, err := os.Stat(testFolder); err == nil { - os.RemoveAll(testFolder) - if err != nil { - Logger.Debug("Could not remove test folder", err) - } + return wrapError("Could not download filter", err) // TODO - I don't think this should break the entire install + } else if downloadPath == "" { + continue // Not a remote filter, skip } // Move filters 'data' folder contents into 'data' - filterName := strings.Split(path.Clean(url), "/")[3] - localDataPath := path.Join(profile.DataPath, filterName) + filterName := filter.GetIdName() + localDataPath := path.Join(p.DataPath, filterName) remoteDataPath := path.Join(downloadPath, "data") // If the filterDataPath already exists, we must not overwrite - // Additionally, if the remote data path doesn't exist, we don't need to do anything + // Additionally, if the remote data path doesn't exist, we don't need + // to do anything if _, err := os.Stat(localDataPath); err == nil { - Logger.Warnf("Filter %s already has data in the 'data' folder. \nYou may manually delete this data and reinstall if you would like these configuration files to be updated.", filterName) + Logger.Warnf("Filter %s already has data in the 'data' folder. \n"+ + "You may manually delete this data and reinstall if you "+ + "would like these configuration files to be updated.", + filterName) } else if _, err := os.Stat(remoteDataPath); err == nil { // Ensure folder exists err = os.MkdirAll(localDataPath, 0666) if err != nil { - Logger.Error("Could not create filter data folder", err) + Logger.Error("Could not create filter data folder", err) // TODO - I don't think this should break the entire install } // Copy 'data' to dataPath - if profile.DataPath != "" { - err = copy.Copy(remoteDataPath, localDataPath, copy.Options{PreserveTimes: false, Sync: false}) + if p.DataPath != "" { + err = copy.Copy( + remoteDataPath, localDataPath, + copy.Options{PreserveTimes: false, Sync: false}) if err != nil { - Logger.Error("Could not initialize filter data", err) + Logger.Error("Could not initialize filter data", err) // TODO - I don't think this should break the entire install } } else { - Logger.Warnf("Filter %s has installation data, but the dataPath is not set. Skipping.", filterName) + Logger.Warnf("Filter %s has installation data, but the "+ + "dataPath is not set. Skipping.", filterName) } } - // Check required files - file, err := ioutil.ReadFile(downloadPath + "/filter.json") + // Create profile from filter.json file + remoteProfile, err := LoadFilterJsonProfile( + filepath.Join(downloadPath, "filter.json"), *filter) if err != nil { - return wrapError(fmt.Sprintf("Couldn't find %s/filter.json!", downloadPath), err) + return err // TODO - I don't think this should break the entire install. Just remove the files and continue. } - // Load subprofile (remote filter) - var remoteProfile Profile - err = json.Unmarshal(file, &remoteProfile) - if err != nil { - return wrapError(fmt.Sprintf("Couldn't load %s/filter.json: ", downloadPath), err) - } - // Propagate venvSlot property - for f := range remoteProfile.Filters { - remoteProfile.Filters[f].VenvSlot = filter.VenvSlot - } - // Install dependencies of remote filters - // recursion ends when there is no more nested remote dependencies - err = InstallDependency(remoteProfile, isForced) + // Install dependencies of remote filters. Recursion ends when there + // is no more nested remote dependencies. + err = remoteProfile.Install(isForced) if err != nil { return err } - // Install filter dependencies + // Install filter dependencies (python/nodejs modules etc.) + // TODO - can we move this out of the loop? This way the filters from + // config.json would also run their install function and we could + // modify the code a little to enable dependencies for them. for _, filter := range remoteProfile.Filters { if filter.RunWith != "" { if f, ok := FilterTypes[filter.RunWith]; ok { err := f.install(filter, downloadPath) if err != nil { - return wrapError(fmt.Sprintf("Couldn't install filter %s", filter.Name), err) + return wrapError(fmt.Sprintf( + "Couldn't install filter %s", filter.Name), err) } } else { - Logger.Warnf("Filter type '%s' not supported", filter.RunWith) + Logger.Warnf( + "Filter type '%s' not supported", filter.RunWith) } } } } return nil } + +// GetFilterPath returns URL for downloading the filter or empty string +// if the filter is not remote +func (f *Filter) GetDownloadUrl() string { + if f.Url != "" { + return FilterNameToUrl(f.Url, f.Filter) + } else if f.Filter != "" { + return FilterNameToUrl(StandardLibraryUrl, f.Filter) + } + return "" +} + +// GetIdName returns the name that identifies the filter. This name is used to +// create and access the data folder for the filter. This property only makes +// sense for remote filters. Non-remote filters return empty string. +func (f *Filter) GetIdName() string { + if f.Filter != "" { + return f.Filter + } else if f.Url != "" { + splitUrl := strings.Split(f.Url, "/") + return splitUrl[len(splitUrl)-1] + } + return "" +} + +// Download downloads the filter and returns the download path. If the filter +// is not remote, it returns empty string. +func (f *Filter) Download(isForced bool) (string, error) { + url := f.GetDownloadUrl() + if url == "" { + return "", nil // No dependency to install + } + + // Download the filter into the cache folder + downloadPath := UrlToPath(url) + + // If downloadPath already exists, we don't need to download it again. + // Force mode allows overwriting. + if _, err := os.Stat(downloadPath); err == nil { + if !isForced { + Logger.Warnf("Dependency %s already installed, skipping. Run "+ + "with '-f' to force.", url) + return "", nil + } else { + Logger.Warnf("Dependency %s already installed, but forcing "+ + "installation.", url) + err := os.RemoveAll(downloadPath) + if err != nil { + return "", wrapError("Could not remove installed filter", err) + } + } + } + + Logger.Infof("Installing dependency %s...", url) + + // Download the filter fresh + ok, err := DownloadGitHubUrl(url, downloadPath) + if err != nil { + Logger.Debug(err) + } + if !ok { + Logger.Debug( + "Failed to download filter " + f.Filter + " without git") + err := getter.Get(downloadPath, url) + if err != nil { + return "", err + } + } + + // Remove 'test' folder, which may be installed via git-getter library + // This is a workaround for cases where our own getter library is not + // able to download the filter. + testFolder := path.Join(downloadPath, "test") + if _, err := os.Stat(testFolder); err == nil { + os.RemoveAll(testFolder) + if err != nil { + Logger.Debug("Could not remove test folder", err) + } + } + return downloadPath, nil +}