Skip to content
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.

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

internal/project: force absolute paths #469

Merged
merged 3 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
53 changes: 32 additions & 21 deletions internal/project/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,25 @@ type Project struct {
logger *zap.Logger
}

// normalizeAndValidatePath makes sure that the path is absolute and
// checks if the path exists.
func normalizeAndValidatePath(path string) (string, error) {
path, err := filepath.Abs(path)
if err != nil {
return "", errors.WithStack(err)
}

if _, err := os.Stat(path); err != nil {
// Handle ErrNotExist to provide more user-friendly error message.
if errors.Is(err, os.ErrNotExist) {
return "", errors.Wrapf(os.ErrNotExist, "failed to open file-based project %q", path)
}
return "", errors.WithStack(err)
}

return path, nil
}

func NewDirProject(
dir string,
opts ...ProjectOption,
Expand All @@ -160,8 +179,11 @@ func NewDirProject(
opt(p)
}

if _, err := os.Stat(dir); err != nil {
return nil, errors.WithStack(err)
var err error

dir, err = normalizeAndValidatePath(dir)
if err != nil {
return nil, errors.Wrapf(err, "failed to open dir-based project %q", dir)
}

p.fs = osfs.New(dir)
Expand All @@ -172,19 +194,18 @@ func NewDirProject(
openOptions = &git.PlainOpenOptions{}
}

var err error
p.repo, err = git.PlainOpenWithOptions(
dir,
openOptions,
)
if err != nil && !errors.Is(err, git.ErrRepositoryNotExists) {
return nil, errors.WithStack(err)
return nil, errors.Wrapf(err, "failed to open dir-based project %q", dir)
}

if p.repo != nil {
wt, err := p.repo.Worktree()
if err != nil {
return nil, errors.WithStack(err)
return nil, errors.Wrapf(err, "failed to open dir-based project %q", dir)
}
p.fs = wt.Filesystem
}
Expand All @@ -202,30 +223,20 @@ func NewFileProject(
) (*Project, error) {
p := &Project{}

// For compatibility, but currently no option is
// valid for file projects,
// For compatibility; many options are not used for file-based projects.
for _, opt := range opts {
opt(p)
}

if !filepath.IsAbs(path) {
var err error
p.filePath, err = filepath.Abs(path)
if err != nil {
return nil, errors.Wrapf(err, "failed to open file-based project %q", path)
}
} else {
p.filePath = path
}
var err error

if _, err := os.Stat(path); err != nil {
// Handle ErrNotExist to provide more user-friendly error message.
if errors.Is(err, os.ErrNotExist) {
return nil, errors.Wrapf(os.ErrNotExist, "failed to open file-based project %q", path)
}
path, err = normalizeAndValidatePath(path)
if err != nil {
return nil, errors.Wrapf(err, "failed to open file-based project %q", path)
}

p.filePath = path

if p.logger == nil {
p.logger = zap.NewNop()
}
Expand Down
28 changes: 27 additions & 1 deletion internal/project/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ func TestNewDirProject(t *testing.T) {
_, err := NewDirProject(unknownDir)
require.ErrorIs(t, err, os.ErrNotExist)
})

t.Run("RelativePathConvertedToAbsolute", func(t *testing.T) {
cwd, err := os.Getwd()
require.NoError(t, err)

projectDir, err := filepath.Rel(cwd, testdata.DirProjectPath())
require.NoError(t, err)

proj, err := NewDirProject(projectDir)
require.NoError(t, err)
assert.True(t, filepath.IsAbs(proj.Root()), "project root is not absolute: %s", proj.Root())
})
}

func TestNewFileProject(t *testing.T) {
Expand All @@ -74,11 +86,23 @@ func TestNewFileProject(t *testing.T) {
require.ErrorIs(t, err, os.ErrNotExist)
})

t.Run("NotAbsPath", func(t *testing.T) {
t.Run("UnknownFileAndRelativePath", func(t *testing.T) {
_, err := NewFileProject("unknown-file.md")
require.ErrorIs(t, err, os.ErrNotExist)
})

t.Run("RelativePathConvertedToAbsolute", func(t *testing.T) {
cwd, err := os.Getwd()
require.NoError(t, err)

fileProject, err := filepath.Rel(cwd, testdata.ProjectFilePath())
require.NoError(t, err)

proj, err := NewFileProject(fileProject)
require.NoError(t, err)
assert.True(t, filepath.IsAbs(proj.Root()), "project root is not absolute: %s", proj.Root())
})

t.Run("ProperFileProject", func(t *testing.T) {
fileProject := testdata.ProjectFilePath()
_, err := NewFileProject(fileProject)
Expand All @@ -92,13 +116,15 @@ func TestProjectRoot(t *testing.T) {
p, err := NewDirProject(gitProjectDir)
require.NoError(t, err)
assert.Equal(t, gitProjectDir, p.Root())
assert.True(t, filepath.IsAbs(p.Root()), "project root is not absolute: %s", p.Root())
})

t.Run("FileProject", func(t *testing.T) {
fileProject := testdata.ProjectFilePath()
p, err := NewFileProject(fileProject)
require.NoError(t, err)
assert.Equal(t, testdata.TestdataPath(), p.Root())
assert.True(t, filepath.IsAbs(p.Root()), "project root is not absolute: %s", p.Root())
})
}

Expand Down
6 changes: 3 additions & 3 deletions testdata/flags/relative.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,6 @@ NAME FILE FIRST COMMAND DESCRIPTION NAMED
hello-js ../../root.md console.log("Hello, runme, from javascript!") It can even run scripting languages. Yes
hello-js-cat ../../root.md console.log("Hello, runme, from javascript!") And it can even run a cell with a custom interpreter. Yes
hello-python ../../root.md def say_hi(): Yes
again ../../very/another.md echo "Hello, runme! Again!" Yes
echo ../../very/deeply/nested/file.md echo "Hello, runme!" Yes
count ../../very/deeply/sub.md echo "1" It can contain multiple lines too. Yes
again ../another.md echo "Hello, runme! Again!" Yes
echo nested/file.md echo "Hello, runme!" Yes
count sub.md echo "1" It can contain multiple lines too. Yes
Loading