Skip to content

refactor(platformifier): use fs.FS and return file map #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pjcdawkins wants to merge 2 commits into
base: refactor/abstract-fs
Choose a base branch
from
Open

refactor(platformifier): use fs.FS and return file map #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e25608a
refactor(platformifier): use fs.FS and return file map
Dec 4, 2023
ac58a58
fix(platformifier): address review feedback
Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions commands/platformify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"errors"
"fmt"
"io"
"os"
"path"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -39,11 +41,7 @@ func Platformify(ctx context.Context, stdout, stderr io.Writer, assets *vendoriz
answers := models.NewAnswers()
answers.Flavor, _ = ctx.Value(FlavorKey).(string)
ctx = models.ToContext(ctx, answers)
ctx = colors.ToContext(
ctx,
stdout,
stderr,
)
ctx = colors.ToContext(ctx, stdout, stderr)
q := questionnaire.New(
&question.WorkingDirectory{},
&question.FilesOverwrite{},
Expand Down Expand Up @@ -76,12 +74,24 @@ func Platformify(ctx context.Context, stdout, stderr io.Writer, assets *vendoriz
input := answers.ToUserInput()

pfier := platformifier.New(input, assets.ConfigFlavor)
err = pfier.Platformify(ctx)
configFiles, err := pfier.Platformify(ctx)
if err != nil {
fmt.Fprintln(stderr, colors.Colorize(colors.ErrorCode, err.Error()))
return fmt.Errorf("could not configure project: %w", err)
}

for file, contents := range configFiles {
filePath := path.Join(answers.Cwd, file)
if err := os.MkdirAll(path.Dir(filePath), os.ModeDir|os.ModePerm); err != nil {
fmt.Fprintf(stderr, "Could not create parent directories of file %s: %s\n", file, err)
continue
}
if err := os.WriteFile(filePath, contents, 0o664); err != nil {
fmt.Fprintf(stderr, "Could not write file %s: %s\n", file, err)
Comment on lines +84 to +90
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code uses the path package (slash-based) to build OS file paths (answers.Cwd + template paths). On Windows, path.Join/path.Dir won’t handle volume names or separators correctly. Prefer filepath.Join/filepath.Dir for OS paths, and use a standard dir/file mode (e.g., 0o755 for MkdirAll and 0o644 for WriteFile) rather than os.ModeDir|os.ModePerm.

Copilot uses AI. Check for mistakes.
continue
Comment on lines +87 to +91
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File write failures are currently logged and then skipped (continue), but the command still proceeds to the "Done" step and returns nil. That can leave the project partially configured without signaling failure. Consider collecting these write errors and returning a non-nil error (or at least failing fast) so callers/users know configuration did not complete successfully.

Suggested change
continue
}
if err := os.WriteFile(filePath, contents, 0o664); err != nil {
fmt.Fprintf(stderr, "Could not write file %s: %s\n", file, err)
continue
return fmt.Errorf("could not create parent directories of file %s: %w", file, err)
}
if err := os.WriteFile(filePath, contents, 0o664); err != nil {
fmt.Fprintf(stderr, "Could not write file %s: %s\n", file, err)
return fmt.Errorf("could not write file %s: %w", file, err)

Copilot uses AI. Check for mistakes.
}
}

done := question.Done{}
return done.Ask(ctx)
}
21 changes: 10 additions & 11 deletions internal/question/application_root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package question
import (
"context"
"path"
"path/filepath"

"github.com/platformsh/platformify/internal/question/models"
"github.com/platformsh/platformify/internal/utils"
Expand All @@ -20,28 +19,28 @@ func (q *ApplicationRoot) Ask(ctx context.Context) error {
for _, dm := range answers.DependencyManagers {
switch dm {
case models.Composer:
if composerPath := utils.FindFile(answers.WorkingDirectory, "composer.json"); composerPath != "" {
answers.ApplicationRoot, _ = filepath.Rel(answers.WorkingDirectory, path.Dir(composerPath))
if composerPath := utils.FindFile(answers.WorkingDirectory, "", "composer.json"); composerPath != "" {
answers.ApplicationRoot = path.Dir(composerPath)
return nil
}
case models.Npm, models.Yarn:
if packagePath := utils.FindFile(answers.WorkingDirectory, "package.json"); packagePath != "" {
answers.ApplicationRoot, _ = filepath.Rel(answers.WorkingDirectory, path.Dir(packagePath))
if packagePath := utils.FindFile(answers.WorkingDirectory, "", "package.json"); packagePath != "" {
answers.ApplicationRoot = path.Dir(packagePath)
return nil
}
case models.Poetry:
if pyProjectPath := utils.FindFile(answers.WorkingDirectory, "pyproject.toml"); pyProjectPath != "" {
answers.ApplicationRoot, _ = filepath.Rel(answers.WorkingDirectory, path.Dir(pyProjectPath))
if pyProjectPath := utils.FindFile(answers.WorkingDirectory, "", "pyproject.toml"); pyProjectPath != "" {
answers.ApplicationRoot = path.Dir(pyProjectPath)
return nil
}
case models.Pipenv:
if pipfilePath := utils.FindFile(answers.WorkingDirectory, "Pipfile"); pipfilePath != "" {
answers.ApplicationRoot, _ = filepath.Rel(answers.WorkingDirectory, path.Dir(pipfilePath))
if pipfilePath := utils.FindFile(answers.WorkingDirectory, "", "Pipfile"); pipfilePath != "" {
answers.ApplicationRoot = path.Dir(pipfilePath)
return nil
}
case models.Pip:
if requirementsPath := utils.FindFile(answers.WorkingDirectory, "requirements.txt"); requirementsPath != "" {
answers.ApplicationRoot, _ = filepath.Rel(answers.WorkingDirectory, path.Dir(requirementsPath))
if requirementsPath := utils.FindFile(answers.WorkingDirectory, "", "requirements.txt"); requirementsPath != "" {
answers.ApplicationRoot = path.Dir(requirementsPath)
return nil
}
}
Expand Down
9 changes: 5 additions & 4 deletions internal/question/build_steps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package question
import (
"context"
"fmt"
"path"
"path/filepath"
"slices"

Expand Down Expand Up @@ -74,8 +73,9 @@ func (q *BuildSteps) Ask(ctx context.Context) error {
)
}
if _, ok := utils.GetJSONValue(
answers.WorkingDirectory,
[]string{"scripts", "build"},
path.Join(answers.WorkingDirectory, "package.json"),
"package.json",
true,
); ok {
if dm == models.Yarn {
Expand All @@ -100,7 +100,8 @@ func (q *BuildSteps) Ask(ctx context.Context) error {
switch answers.Stack {
case models.Django:
if managePyPath := utils.FindFile(
path.Join(answers.WorkingDirectory, answers.ApplicationRoot),
answers.WorkingDirectory,
answers.ApplicationRoot,
managePyFile,
); managePyPath != "" {
prefix := ""
Expand All @@ -110,7 +111,7 @@ func (q *BuildSteps) Ask(ctx context.Context) error {
prefix = "poetry run "
}

managePyPath, _ = filepath.Rel(path.Join(answers.WorkingDirectory, answers.ApplicationRoot), managePyPath)
managePyPath, _ = filepath.Rel(answers.ApplicationRoot, managePyPath)
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

managePyPath and answers.ApplicationRoot are fs paths (slash-separated) used to generate commands that run in a Linux container. Using filepath.Rel here can produce OS-specific separators (e.g., backslashes on Windows), yielding an invalid path in the generated command. Prefer path.Rel (or otherwise force '/' separators) for these in-repo paths.

Copilot uses AI. Check for mistakes.
assets, _ := vendorization.FromContext(ctx)
answers.BuildSteps = append(
answers.BuildSteps,
Expand Down
5 changes: 5 additions & 0 deletions internal/question/build_steps_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"reflect"
"testing"
"testing/fstest"

"github.com/platformsh/platformify/internal/question/models"
)
Expand Down Expand Up @@ -37,6 +38,7 @@ func TestBuildSteps_Ask(t *testing.T) {
Dependencies: map[string]map[string]string{},
DependencyManagers: []models.DepManager{models.Yarn},
Environment: map[string]string{},
WorkingDirectory: fstest.MapFS{},
}},
buildSteps: []string{"yarn", "yarn exec next build"},
wantErr: false,
Expand All @@ -50,6 +52,7 @@ func TestBuildSteps_Ask(t *testing.T) {
Dependencies: map[string]map[string]string{},
DependencyManagers: []models.DepManager{models.Npm},
Environment: map[string]string{},
WorkingDirectory: fstest.MapFS{},
}},
buildSteps: []string{"npm i", "npm exec next build"},
wantErr: false,
Expand All @@ -63,6 +66,7 @@ func TestBuildSteps_Ask(t *testing.T) {
Dependencies: map[string]map[string]string{},
DependencyManagers: []models.DepManager{models.Bundler},
Environment: map[string]string{},
WorkingDirectory: fstest.MapFS{},
}},
buildSteps: []string{"bundle install"},
wantErr: false,
Expand All @@ -76,6 +80,7 @@ func TestBuildSteps_Ask(t *testing.T) {
Dependencies: map[string]map[string]string{},
DependencyManagers: []models.DepManager{models.Bundler},
Environment: map[string]string{},
WorkingDirectory: fstest.MapFS{},
}},
buildSteps: []string{"bundle install", "bundle exec rails assets:precompile"},
wantErr: false,
Expand Down
14 changes: 7 additions & 7 deletions internal/question/dependency_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,27 @@ func (q *DependencyManager) Ask(ctx context.Context) error {
}
}()

if exists := utils.FileExists(answers.WorkingDirectory, poetryLockFile); exists {
if exists := utils.FileExists(answers.WorkingDirectory, "", poetryLockFile); exists {
answers.DependencyManagers = append(answers.DependencyManagers, models.Poetry)
} else if exists := utils.FileExists(answers.WorkingDirectory, pipenvLockFile); exists {
} else if exists := utils.FileExists(answers.WorkingDirectory, "", pipenvLockFile); exists {
answers.DependencyManagers = append(answers.DependencyManagers, models.Pipenv)
} else if exists := utils.FileExists(answers.WorkingDirectory, pipLockFile); exists {
} else if exists := utils.FileExists(answers.WorkingDirectory, "", pipLockFile); exists {
answers.DependencyManagers = append(answers.DependencyManagers, models.Pip)
}

if exists := utils.FileExists(answers.WorkingDirectory, composerLockFile); exists {
if exists := utils.FileExists(answers.WorkingDirectory, "", composerLockFile); exists {
answers.DependencyManagers = append(answers.DependencyManagers, models.Composer)
answers.Dependencies["php"] = map[string]string{"composer/composer": "^2"}
}

if exists := utils.FileExists(answers.WorkingDirectory, yarnLockFileName); exists {
if exists := utils.FileExists(answers.WorkingDirectory, "", yarnLockFileName); exists {
answers.DependencyManagers = append(answers.DependencyManagers, models.Yarn)
answers.Dependencies["nodejs"] = map[string]string{"yarn": "^1.22.0"}
} else if exists := utils.FileExists(answers.WorkingDirectory, npmLockFileName); exists {
} else if exists := utils.FileExists(answers.WorkingDirectory, "", npmLockFileName); exists {
answers.DependencyManagers = append(answers.DependencyManagers, models.Npm)
}

if exists := utils.FileExists(answers.WorkingDirectory, bundlerLockFile); exists {
if exists := utils.FileExists(answers.WorkingDirectory, "", bundlerLockFile); exists {
answers.DependencyManagers = append(answers.DependencyManagers, models.Bundler)
}

Expand Down
5 changes: 2 additions & 3 deletions internal/question/deploy_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package question
import (
"context"
"fmt"
"path"
"path/filepath"
"slices"

Expand All @@ -21,9 +20,9 @@ func (q *DeployCommand) Ask(ctx context.Context) error {

switch answers.Stack {
case models.Django:
managePyPath := utils.FindFile(path.Join(answers.WorkingDirectory, answers.ApplicationRoot), managePyFile)
managePyPath := utils.FindFile(answers.WorkingDirectory, answers.ApplicationRoot, managePyFile)
if managePyPath != "" {
managePyPath, _ = filepath.Rel(path.Join(answers.WorkingDirectory, answers.ApplicationRoot), managePyPath)
managePyPath, _ = filepath.Rel(answers.ApplicationRoot, managePyPath)
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

managePyPath and answers.ApplicationRoot are fs paths intended for use inside the generated deploy command (Linux container). filepath.Rel can introduce OS-specific separators (notably '\' on Windows), producing an invalid command path. Use path.Rel or normalize to '/' before formatting the command.

Suggested change
managePyPath, _ = filepath.Rel(answers.ApplicationRoot, managePyPath)
managePyPath, _ = filepath.Rel(answers.ApplicationRoot, managePyPath)
managePyPath = filepath.ToSlash(managePyPath)

Copilot uses AI. Check for mistakes.
prefix := ""
if slices.Contains(answers.DependencyManagers, models.Pipenv) {
prefix = "pipenv run "
Expand Down
2 changes: 1 addition & 1 deletion internal/question/done.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (q *Done) Ask(ctx context.Context) error {
),
),
)
fmt.Fprintf(out, " $ git init %s\n", answers.WorkingDirectory)
fmt.Fprintf(out, " $ git init %s\n", answers.Cwd)
fmt.Fprintln(out, " $ git add .")
fmt.Fprintf(out, " $ git commit -m 'Add %s configuration files'\n", assets.ServiceName)
fmt.Fprintf(out, " $ %s project:set-remote\n", assets.Binary)
Expand Down
7 changes: 3 additions & 4 deletions internal/question/files_overwrite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ package question
import (
"context"
"fmt"
"os"
"path/filepath"
"io/fs"

"github.com/AlecAivazis/survey/v2"

Expand All @@ -30,7 +29,7 @@ func (q *FilesOverwrite) Ask(ctx context.Context) error {
assets, _ := vendorization.FromContext(ctx)
existingFiles := make([]string, 0, len(assets.ProprietaryFiles()))
for _, p := range assets.ProprietaryFiles() {
if st, err := os.Stat(filepath.Join(answers.WorkingDirectory, p)); err == nil && !st.IsDir() {
if st, err := fs.Stat(answers.WorkingDirectory, p); err == nil && !st.IsDir() {
existingFiles = append(existingFiles, p)
}
}
Expand All @@ -40,7 +39,7 @@ func (q *FilesOverwrite) Ask(ctx context.Context) error {
stderr,
colors.Colorize(
colors.WarningCode,
fmt.Sprintf("You are reconfiguring the project at %s.", answers.WorkingDirectory),
fmt.Sprintf("You are reconfiguring the project at %s.", answers.Cwd),
),
)
fmt.Fprintln(
Expand Down
9 changes: 4 additions & 5 deletions internal/question/locations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package question

import (
"context"
"path/filepath"
"path"

"github.com/platformsh/platformify/internal/question/models"
"github.com/platformsh/platformify/internal/utils"
Expand All @@ -29,10 +29,9 @@ func (q *Locations) Ask(ctx context.Context) error {
"passthru": "/index.php",
"root": "",
}
if indexPath := utils.FindFile(answers.WorkingDirectory, "index.php"); indexPath != "" {
indexRelPath, _ := filepath.Rel(answers.WorkingDirectory, indexPath)
if filepath.Dir(indexRelPath) != "." {
locations["root"] = filepath.Dir(indexRelPath)
if indexPath := utils.FindFile(answers.WorkingDirectory, "", "index.php"); indexPath != "" {
if path.Dir(indexPath) != "." {
locations["root"] = path.Dir(indexPath)
}
}
answers.Locations["/"] = locations
Expand Down
37 changes: 19 additions & 18 deletions internal/question/models/answer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package models

import (
"encoding/json"
"io/fs"
"os"
"path/filepath"
"strings"
Expand All @@ -10,23 +11,24 @@ import (
)

type Answers struct {
Stack Stack `json:"stack"`
Flavor string `json:"flavor"`
Type RuntimeType `json:"type"`
Name string `json:"name"`
ApplicationRoot string `json:"application_root"`
Environment map[string]string `json:"environment"`
BuildSteps []string `json:"build_steps"`
WebCommand string `json:"web_command"`
SocketFamily SocketFamily `json:"socket_family"`
DeployCommand []string `json:"deploy_command"`
DependencyManagers []DepManager `json:"dependency_managers"`
Dependencies map[string]map[string]string `json:"dependencies"`
BuildFlavor string `json:"build_flavor"`
Disk string `json:"disk"`
Mounts map[string]map[string]string `json:"mounts"`
Services []Service `json:"services"`
WorkingDirectory string `json:"working_directory"`
Stack Stack `json:"stack"`
Flavor string `json:"flavor"`
Type RuntimeType `json:"type"`
Name string `json:"name"`
ApplicationRoot string `json:"application_root"`
Environment map[string]string `json:"environment"`
BuildSteps []string `json:"build_steps"`
WebCommand string `json:"web_command"`
SocketFamily SocketFamily `json:"socket_family"`
DeployCommand []string `json:"deploy_command"`
DependencyManagers []DepManager `json:"dependency_managers"`
Dependencies map[string]map[string]string `json:"dependencies"`
BuildFlavor string `json:"build_flavor"`
Disk string `json:"disk"`
Mounts map[string]map[string]string `json:"mounts"`
Services []Service `json:"services"`
WorkingDirectory fs.FS
Cwd string
Comment on lines +30 to +31
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answers.WorkingDirectory is now fs.FS and Cwd is a new field, but neither has a json tag. If Answers is ever marshaled/unmarshaled, this will either serialize an implementation-specific value for WorkingDirectory or change the JSON shape unexpectedly (Cwd becomes "Cwd"). Consider tagging WorkingDirectory as json:"-" and adding an explicit tag for Cwd (e.g., json:"cwd") to avoid accidental persistence/API changes.

Suggested change
WorkingDirectory fs.FS
Cwd string
WorkingDirectory fs.FS `json:"-"`
Cwd string `json:"cwd"`

Copilot uses AI. Check for mistakes.
HasGit bool `json:"has_git"`
FilesCreated []string `json:"files_created"`
Locations map[string]map[string]interface{} `json:"locations"`
Expand Down Expand Up @@ -115,7 +117,6 @@ func (a *Answers) ToUserInput() *platformifier.UserInput {

return &platformifier.UserInput{
Stack: getStack(a.Stack),
Root: "",
ApplicationRoot: filepath.Join(string(os.PathSeparator), a.ApplicationRoot),
Name: a.Name,
Comment on lines 119 to 121
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToUserInput currently prefixes ApplicationRoot with an OS path separator (e.g., "/app" or "\app"). With the new fs.FS-based file discovery (fs.WalkDir/os.DirFS), leading separators (and Windows backslashes) make paths invalid for fs.FS operations and can prevent stack-specific platformifiers from finding project files. Consider keeping ApplicationRoot as a slash-separated relative path for fs traversal, and only rendering a leading "/" (if needed) at template time.

Copilot uses AI. Check for mistakes.
Type: a.Type.String(),
Expand Down
2 changes: 1 addition & 1 deletion internal/question/name.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (q *Name) Ask(ctx context.Context) error {
}

question := &survey.Input{
Message: "Tell us your project's application name:", Default: slugify(path.Base(answers.WorkingDirectory)),
Message: "Tell us your project's application name:", Default: slugify(path.Base(answers.Cwd)),
}
Comment on lines +33 to 34
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

answers.Cwd is an OS path, but path.Base is for slash-separated paths and won’t correctly handle Windows paths (backslashes/volume names). Prefer filepath.Base here to derive the default name portably.

Copilot uses AI. Check for mistakes.

var name string
Expand Down
Loading
Loading