diff --git a/cmd/download_test.go b/cmd/download_test.go index 7beb26ae1..f9aeacebe 100644 --- a/cmd/download_test.go +++ b/cmd/download_test.go @@ -142,8 +142,8 @@ func TestDownload(t *testing.T) { targetDir := filepath.Join(tmpDir, tc.expectedDir) assertDownloadedCorrectFiles(t, targetDir) - path := filepath.Join(targetDir, "bogus-track", "bogus-exercise", ".solution.json") - b, err := ioutil.ReadFile(path) + dir := filepath.Join(targetDir, "bogus-track", "bogus-exercise") + b, err := ioutil.ReadFile(workspace.NewExerciseFromDir(dir).MetadataFilepath()) var s workspace.Solution err = json.Unmarshal(b, &s) assert.NoError(t, err) diff --git a/cmd/submit.go b/cmd/submit.go index 29d7eb440..8e364427a 100644 --- a/cmd/submit.go +++ b/cmd/submit.go @@ -129,7 +129,13 @@ func runSubmit(cfg config.Config, flags *pflag.FlagSet, args []string) error { } exercise := workspace.NewExerciseFromDir(exerciseDir) - + migrationStatus, err := exercise.MigrateLegacyMetadataFile() + if err != nil { + return err + } + if verbose, _ := flags.GetBool("verbose"); verbose { + fmt.Fprintf(os.Stderr, migrationStatus.String()) + } solution, err := workspace.NewSolution(exerciseDir) if err != nil { return err diff --git a/cmd/submit_test.go b/cmd/submit_test.go index ca1147e32..c651a2ce3 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -1,6 +1,7 @@ package cmd import ( + "encoding/json" "io/ioutil" "net/http" "net/http/httptest" @@ -185,6 +186,69 @@ func TestSubmitFiles(t *testing.T) { assert.Equal(t, "This is the readme.", submittedFiles["README.md"]) } +func TestLegacySolutionMetadataMigration(t *testing.T) { + oldOut := Out + oldErr := Err + Out = ioutil.Discard + Err = ioutil.Discard + defer func() { + Out = oldOut + Err = oldErr + }() + // The fake endpoint will populate this when it receives the call from the command. + submittedFiles := map[string]string{} + ts := fakeSubmitServer(t, submittedFiles) + defer ts.Close() + + tmpDir, err := ioutil.TempDir("", "legacy-metadata-file") + defer os.RemoveAll(tmpDir) + assert.NoError(t, err) + + dir := filepath.Join(tmpDir, "bogus-track", "bogus-exercise") + os.MkdirAll(dir, os.FileMode(0755)) + + solution := &workspace.Solution{ + ID: "bogus-solution-uuid", + Track: "bogus-track", + Exercise: "bogus-exercise", + URL: "http://example.com/bogus-url", + IsRequester: true, + } + b, err := json.Marshal(solution) + assert.NoError(t, err) + exercise := workspace.NewExerciseFromDir(dir) + err = ioutil.WriteFile(exercise.LegacyMetadataFilepath(), b, os.FileMode(0600)) + assert.NoError(t, err) + + file := filepath.Join(dir, "file.txt") + err = ioutil.WriteFile(file, []byte("This is a file."), os.FileMode(0755)) + assert.NoError(t, err) + + v := viper.New() + v.Set("token", "abc123") + v.Set("workspace", tmpDir) + v.Set("apibaseurl", ts.URL) + cfg := config.Config{ + Persister: config.InMemoryPersister{}, + Dir: tmpDir, + UserViperConfig: v, + } + + ok, _ := exercise.HasLegacyMetadata() + assert.True(t, ok) + ok, _ = exercise.HasMetadata() + assert.False(t, ok) + + err = runSubmit(cfg, pflag.NewFlagSet("fake", pflag.PanicOnError), []string{file}) + assert.NoError(t, err) + assert.Equal(t, "This is a file.", submittedFiles["file.txt"]) + + ok, _ = exercise.HasLegacyMetadata() + assert.False(t, ok) + ok, _ = exercise.HasMetadata() + assert.True(t, ok) +} + func TestSubmitWithEmptyFile(t *testing.T) { oldOut := Out oldErr := Err diff --git a/fixtures/is-solution-path/broken/.solution.json b/fixtures/is-solution-path/broken/.exercism/solution.json similarity index 100% rename from fixtures/is-solution-path/broken/.solution.json rename to fixtures/is-solution-path/broken/.exercism/solution.json diff --git a/fixtures/is-solution-path/yepp/.solution.json b/fixtures/is-solution-path/yepp/.exercism/solution.json similarity index 100% rename from fixtures/is-solution-path/yepp/.solution.json rename to fixtures/is-solution-path/yepp/.exercism/solution.json diff --git a/fixtures/solution-dir/workspace/exercise/.solution.json b/fixtures/solution-dir/workspace/exercise/.exercism/solution.json similarity index 100% rename from fixtures/solution-dir/workspace/exercise/.solution.json rename to fixtures/solution-dir/workspace/exercise/.exercism/solution.json diff --git a/fixtures/solution-path/creatures/gazelle-2/.solution.json b/fixtures/solution-path/creatures/gazelle-2/.exercism/solution.json similarity index 100% rename from fixtures/solution-path/creatures/gazelle-2/.solution.json rename to fixtures/solution-path/creatures/gazelle-2/.exercism/solution.json diff --git a/fixtures/solution-path/creatures/gazelle-3/.solution.json b/fixtures/solution-path/creatures/gazelle-3/.exercism/solution.json similarity index 100% rename from fixtures/solution-path/creatures/gazelle-3/.solution.json rename to fixtures/solution-path/creatures/gazelle-3/.exercism/solution.json diff --git a/fixtures/solution-path/creatures/gazelle/.solution.json b/fixtures/solution-path/creatures/gazelle/.exercism/solution.json similarity index 100% rename from fixtures/solution-path/creatures/gazelle/.solution.json rename to fixtures/solution-path/creatures/gazelle/.exercism/solution.json diff --git a/fixtures/solutions/alpha/.solution.json b/fixtures/solutions/alpha/.exercism/solution.json similarity index 100% rename from fixtures/solutions/alpha/.solution.json rename to fixtures/solutions/alpha/.exercism/solution.json diff --git a/fixtures/solutions/bravo/.solution.json b/fixtures/solutions/bravo/.exercism/solution.json similarity index 100% rename from fixtures/solutions/bravo/.solution.json rename to fixtures/solutions/bravo/.exercism/solution.json diff --git a/fixtures/solutions/charlie/.solution.json b/fixtures/solutions/charlie/.exercism/solution.json similarity index 100% rename from fixtures/solutions/charlie/.solution.json rename to fixtures/solutions/charlie/.exercism/solution.json diff --git a/visibility/hide_file.go b/visibility/hide_file.go deleted file mode 100644 index 7fc015772..000000000 --- a/visibility/hide_file.go +++ /dev/null @@ -1,13 +0,0 @@ -// +build !windows - -package visibility - -// HideFile is a no-op for non-Windows systems. -func HideFile(string) error { - return nil -} - -// ShowFile is a no-op for non-Windows systems. -func ShowFile(path string) error { - return nil -} diff --git a/visibility/hide_file_windows.go b/visibility/hide_file_windows.go deleted file mode 100644 index c565e5703..000000000 --- a/visibility/hide_file_windows.go +++ /dev/null @@ -1,41 +0,0 @@ -package visibility - -import "syscall" - -// HideFile sets a Windows file's 'hidden' attribute. -// This is the equivalent of giving a filename on -// Linux or MacOS a leading dot (e.g. .bash_rc). -func HideFile(path string) error { - return setVisibility(path, false) -} - -// ShowFile unsets a Windows file's 'hidden' attribute. -func ShowFile(path string) error { - return setVisibility(path, true) -} - -func setVisibility(path string, visible bool) error { - // This is based on the discussion in - // https://www.reddit.com/r/golang/comments/5t3ezd/hidden_files_directories/ - // but instead of duplicating all the effort to write the file, this takes - // the path of a written file and then flips the bit on the relevant attribute. - // The attributes are a bitmask (uint32), so we can't call - // SetFileAttributes(ptr, syscall.File_ATTRIBUTE_HIDDEN) as suggested, since - // that would wipe out any existing attributes. - ptr, err := syscall.UTF16PtrFromString(path) - if err != nil { - return err - } - attributes, err := syscall.GetFileAttributes(ptr) - if err != nil { - return err - } - - if visible { - attributes &^= syscall.FILE_ATTRIBUTE_HIDDEN - } else { - attributes |= syscall.FILE_ATTRIBUTE_HIDDEN - } - - return syscall.SetFileAttributes(ptr, attributes) -} diff --git a/workspace/exercise.go b/workspace/exercise.go index 9c3815e66..34dc31596 100644 --- a/workspace/exercise.go +++ b/workspace/exercise.go @@ -37,7 +37,12 @@ func (e Exercise) Filepath() string { // MetadataFilepath is the absolute path to the exercise metadata. func (e Exercise) MetadataFilepath() string { - return filepath.Join(e.Filepath(), solutionFilename) + return filepath.Join(e.Filepath(), metadataFilepath) +} + +// LegacyMetadataFilepath is the absolute path to the legacy exercise metadata. +func (e Exercise) LegacyMetadataFilepath() string { + return filepath.Join(e.Filepath(), legacySolutionFilename) } // MetadataDir returns the directory that the exercise metadata lives in. @@ -59,3 +64,59 @@ func (e Exercise) HasMetadata() (bool, error) { } return false, err } + +// HasLegacyMetadata checks for the presence of a legacy exercise metadata file. +// If there is no such file, it could also be an unrelated directory. +func (e Exercise) HasLegacyMetadata() (bool, error) { + _, err := os.Lstat(e.LegacyMetadataFilepath()) + if os.IsNotExist(err) { + return false, nil + } + if err == nil { + return true, nil + } + return false, err +} + +// MigrationStatus represents the result of migrating a legacy metadata file. +type MigrationStatus int + +// MigrationStatus +const ( + MigrationStatusNoop MigrationStatus = iota + MigrationStatusMigrated + MigrationStatusRemoved +) + +func (m MigrationStatus) String() string { + switch m { + case MigrationStatusMigrated: + return "\nMigrated metadata\n" + case MigrationStatusRemoved: + return "\nRemoved legacy metadata\n" + default: + return "" + } +} + +// MigrateLegacyMetadataFile migrates a legacy metadata file to the modern location. +// This is a noop if the metadata file isn't legacy. +// If both legacy and modern metadata files exist, the legacy file will be deleted. +func (e Exercise) MigrateLegacyMetadataFile() (MigrationStatus, error) { + if ok, _ := e.HasLegacyMetadata(); !ok { + return MigrationStatusNoop, nil + } + if err := os.MkdirAll(filepath.Dir(e.MetadataFilepath()), os.FileMode(0755)); err != nil { + return MigrationStatusNoop, err + } + if ok, _ := e.HasMetadata(); !ok { + if err := os.Rename(e.LegacyMetadataFilepath(), e.MetadataFilepath()); err != nil { + return MigrationStatusNoop, err + } + return MigrationStatusMigrated, nil + } + if err := os.Remove(e.LegacyMetadataFilepath()); err != nil { + return MigrationStatusNoop, err + } + return MigrationStatusRemoved, nil +} diff --git a/workspace/exercise_test.go b/workspace/exercise_test.go index c3d54a5d7..0a6d1d8fa 100644 --- a/workspace/exercise_test.go +++ b/workspace/exercise_test.go @@ -17,9 +17,9 @@ func TestHasMetadata(t *testing.T) { exerciseA := Exercise{Root: ws, Track: "bogus-track", Slug: "apple"} exerciseB := Exercise{Root: ws, Track: "bogus-track", Slug: "banana"} - err = os.MkdirAll(filepath.Join(exerciseA.Filepath()), os.FileMode(0755)) + err = os.MkdirAll(filepath.Dir(exerciseA.MetadataFilepath()), os.FileMode(0755)) assert.NoError(t, err) - err = os.MkdirAll(filepath.Join(exerciseB.Filepath()), os.FileMode(0755)) + err = os.MkdirAll(filepath.Dir(exerciseB.MetadataFilepath()), os.FileMode(0755)) assert.NoError(t, err) err = ioutil.WriteFile(exerciseA.MetadataFilepath(), []byte{}, os.FileMode(0600)) @@ -34,6 +34,31 @@ func TestHasMetadata(t *testing.T) { assert.False(t, ok) } +func TestHasLegacyMetadata(t *testing.T) { + ws, err := ioutil.TempDir("", "fake-workspace") + defer os.RemoveAll(ws) + assert.NoError(t, err) + + exerciseA := Exercise{Root: ws, Track: "bogus-track", Slug: "apple"} + exerciseB := Exercise{Root: ws, Track: "bogus-track", Slug: "banana"} + + err = os.MkdirAll(filepath.Dir(exerciseA.LegacyMetadataFilepath()), os.FileMode(0755)) + assert.NoError(t, err) + err = os.MkdirAll(filepath.Dir(exerciseB.LegacyMetadataFilepath()), os.FileMode(0755)) + assert.NoError(t, err) + + err = ioutil.WriteFile(exerciseA.LegacyMetadataFilepath(), []byte{}, os.FileMode(0600)) + assert.NoError(t, err) + + ok, err := exerciseA.HasLegacyMetadata() + assert.NoError(t, err) + assert.True(t, ok) + + ok, err = exerciseB.HasLegacyMetadata() + assert.NoError(t, err) + assert.False(t, ok) +} + func TestNewFromDir(t *testing.T) { dir := filepath.Join("something", "another", "whatever", "the-track", "the-exercise") @@ -42,3 +67,99 @@ func TestNewFromDir(t *testing.T) { assert.Equal(t, "the-track", exercise.Track) assert.Equal(t, "the-exercise", exercise.Slug) } + +func TestMigrationStatusString(t *testing.T) { + assert.Equal(t, "\nMigrated metadata\n", MigrationStatusMigrated.String()) + assert.Equal(t, "\nRemoved legacy metadata\n", MigrationStatusRemoved.String()) + assert.Equal(t, "", MigrationStatusNoop.String()) + assert.Equal(t, "", MigrationStatus(-1).String()) +} + +func TestMigrateLegacyMetadataFileWithoutLegacy(t *testing.T) { + ws, err := ioutil.TempDir("", "fake-workspace") + defer os.RemoveAll(ws) + assert.NoError(t, err) + + exercise := Exercise{Root: ws, Track: "bogus-track", Slug: "no-legacy"} + metadataFilepath := exercise.MetadataFilepath() + err = os.MkdirAll(filepath.Dir(metadataFilepath), os.FileMode(0755)) + assert.NoError(t, err) + + err = ioutil.WriteFile(metadataFilepath, []byte{}, os.FileMode(0600)) + assert.NoError(t, err) + + ok, _ := exercise.HasLegacyMetadata() + assert.False(t, ok) + ok, _ = exercise.HasMetadata() + assert.True(t, ok) + + status, err := exercise.MigrateLegacyMetadataFile() + assert.Equal(t, MigrationStatusNoop, status) + assert.NoError(t, err) + + ok, _ = exercise.HasLegacyMetadata() + assert.False(t, ok) + ok, _ = exercise.HasMetadata() + assert.True(t, ok) +} + +func TestMigrateLegacyMetadataFileWithLegacy(t *testing.T) { + ws, err := ioutil.TempDir("", "fake-workspace") + defer os.RemoveAll(ws) + assert.NoError(t, err) + + exercise := Exercise{Root: ws, Track: "bogus-track", Slug: "legacy"} + legacyMetadataFilepath := exercise.LegacyMetadataFilepath() + err = os.MkdirAll(filepath.Dir(legacyMetadataFilepath), os.FileMode(0755)) + assert.NoError(t, err) + + err = ioutil.WriteFile(legacyMetadataFilepath, []byte{}, os.FileMode(0600)) + assert.NoError(t, err) + + ok, _ := exercise.HasLegacyMetadata() + assert.True(t, ok) + ok, _ = exercise.HasMetadata() + assert.False(t, ok) + + status, err := exercise.MigrateLegacyMetadataFile() + assert.Equal(t, MigrationStatusMigrated, status) + assert.NoError(t, err) + + ok, _ = exercise.HasLegacyMetadata() + assert.False(t, ok) + ok, _ = exercise.HasMetadata() + assert.True(t, ok) +} + +func TestMigrateLegacyMetadataFileWithLegacyAndModern(t *testing.T) { + ws, err := ioutil.TempDir("", "fake-workspace") + defer os.RemoveAll(ws) + assert.NoError(t, err) + + exercise := Exercise{Root: ws, Track: "bogus-track", Slug: "both-legacy-and-modern"} + metadataFilepath := exercise.MetadataFilepath() + legacyMetadataFilepath := exercise.LegacyMetadataFilepath() + err = os.MkdirAll(filepath.Dir(legacyMetadataFilepath), os.FileMode(0755)) + assert.NoError(t, err) + err = os.MkdirAll(filepath.Dir(metadataFilepath), os.FileMode(0755)) + assert.NoError(t, err) + + err = ioutil.WriteFile(legacyMetadataFilepath, []byte{}, os.FileMode(0600)) + assert.NoError(t, err) + err = ioutil.WriteFile(metadataFilepath, []byte{}, os.FileMode(0600)) + assert.NoError(t, err) + + ok, _ := exercise.HasLegacyMetadata() + assert.True(t, ok) + ok, _ = exercise.HasMetadata() + assert.True(t, ok) + + status, err := exercise.MigrateLegacyMetadataFile() + assert.Equal(t, MigrationStatusRemoved, status) + assert.NoError(t, err) + + ok, _ = exercise.HasLegacyMetadata() + assert.False(t, ok) + ok, _ = exercise.HasMetadata() + assert.True(t, ok) +} diff --git a/workspace/solution.go b/workspace/solution.go index 518f819f3..a5722d6c9 100644 --- a/workspace/solution.go +++ b/workspace/solution.go @@ -8,11 +8,13 @@ import ( "path/filepath" "strings" "time" - - "github.com/exercism/cli/visibility" ) -const solutionFilename = ".solution.json" +const solutionFilename = "solution.json" +const legacySolutionFilename = ".solution.json" +const ignoreSubdir = ".exercism" + +var metadataFilepath = filepath.Join(ignoreSubdir, solutionFilename) // Solution contains metadata about a user's solution. type Solution struct { @@ -30,8 +32,7 @@ type Solution struct { // NewSolution reads solution metadata from a file in the given directory. func NewSolution(dir string) (*Solution, error) { - path := filepath.Join(dir, solutionFilename) - b, err := ioutil.ReadFile(path) + b, err := ioutil.ReadFile(filepath.Join(dir, metadataFilepath)) if err != nil { return &Solution{}, err } @@ -67,17 +68,15 @@ func (s *Solution) Write(dir string) error { if err != nil { return err } - - path := filepath.Join(dir, solutionFilename) - - // Hack because ioutil.WriteFile fails on hidden files - visibility.ShowFile(path) - - if err := ioutil.WriteFile(path, b, os.FileMode(0600)); err != nil { + metadataAbsoluteFilepath := filepath.Join(dir, metadataFilepath) + if err = os.MkdirAll(filepath.Dir(metadataAbsoluteFilepath), os.FileMode(0755)); err != nil { + return err + } + if err = ioutil.WriteFile(metadataAbsoluteFilepath, b, os.FileMode(0600)); err != nil { return err } s.Dir = dir - return visibility.HideFile(path) + return nil } // PathToParent is the relative path from the workspace to the parent dir. diff --git a/workspace/workspace.go b/workspace/workspace.go index 843953013..78c20b65b 100644 --- a/workspace/workspace.go +++ b/workspace/workspace.go @@ -110,7 +110,10 @@ func (ws Workspace) SolutionDir(s string) (string, error) { if _, err := os.Lstat(path); os.IsNotExist(err) { return "", err } - if _, err := os.Lstat(filepath.Join(path, solutionFilename)); err == nil { + if _, err := os.Lstat(filepath.Join(path, metadataFilepath)); err == nil { + return path, nil + } + if _, err := os.Lstat(filepath.Join(path, legacySolutionFilename)); err == nil { return path, nil } path = filepath.Dir(path) diff --git a/workspace/workspace_test.go b/workspace/workspace_test.go index b48f3cf35..322f04ebb 100644 --- a/workspace/workspace_test.go +++ b/workspace/workspace_test.go @@ -60,11 +60,12 @@ func TestWorkspaceExercises(t *testing.T) { b2 := filepath.Join(tmpDir, "track-b", "exercise-two") for _, path := range []string{a1, a2, b1, b2} { - err := os.MkdirAll(path, os.FileMode(0755)) + metadataAbsoluteFilepath := filepath.Join(path, metadataFilepath) + err := os.MkdirAll(filepath.Dir(metadataAbsoluteFilepath), os.FileMode(0755)) assert.NoError(t, err) if path != a2 { - err = ioutil.WriteFile(filepath.Join(path, solutionFilename), []byte{}, os.FileMode(0600)) + err = ioutil.WriteFile(metadataAbsoluteFilepath, []byte{}, os.FileMode(0600)) assert.NoError(t, err) } }