这是indexloc提供的服务，不要输入任何密码
Skip to content

Enable inputs to be relative again. #1937

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

Merged
merged 3 commits into from
Sep 13, 2022
+48 −2
Merged

Enable inputs to be relative again. #1937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b915bc4
Enable inputs to be relative again.
nathanhammond Sep 13, 2022
6f11d91
Merge branch 'main' into relative-inputs
tknickman Sep 13, 2022
7f680e6
Merge branch 'main' into relative-inputs
tknickman Sep 13, 2022
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
17 changes: 15 additions & 2 deletions cli/internal/hashing/package_deps_hash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func GetPackageDeps(rootPath turbopath.AbsolutePath, p *PackageDepsOptions) (map
// Add all the checked in hashes.
var result map[turbopath.AnchoredUnixPath]string

// make a copy of the inputPatterns array, becase we may be appending to it later.
// make a copy of the inputPatterns array, because we may be appending to it later.
calculatedInputs := make([]string, len(p.InputPatterns))
copy(calculatedInputs, p.InputPatterns)

Expand All @@ -50,7 +50,20 @@ func GetPackageDeps(rootPath turbopath.AbsolutePath, p *PackageDepsOptions) (map
// Note this package.json will be resolved relative to the pkgPath.
calculatedInputs = append(calculatedInputs, "package.json")

absoluteFilesToHash, err := globby.GlobFiles(pkgPath.ToStringDuringMigration(), calculatedInputs, nil)
// The input patterns are relative to the package.
// However, we need to change the globbing to be relative to the repo root.
// Prepend the package path to each of the input patterns.
prefixedInputPatterns := make([]string, len(calculatedInputs))
for index, pattern := range calculatedInputs {
rerooted, err := rootPath.PathTo(pkgPath.Join(pattern))
if err != nil {
return nil, err
}
prefixedInputPatterns[index] = rerooted
}
Comment on lines +57 to +63
Copy link
Contributor

@dglsparsons dglsparsons Sep 13, 2022

Choose a reason for hiding this comment

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

If calculatedInputs is already a copy made within this function, could we avoid allocating again for prefixedInputPatterns here and just mutate calculatedInputs?

Copy link
Contributor Author

@nathanhammond nathanhammond Sep 13, 2022

Choose a reason for hiding this comment

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

We're using the unprefixed version in a different location just a little bit later. There is some alignment to do here, but I'm planning to delay that until we get to refactoring all of our globbing code in a single pass. (At which point I'm imagining it will become aware of "repo root" and "package root" concepts, making this irrelevant.)

Copy link
Contributor Author

@nathanhammond nathanhammond Sep 13, 2022

Choose a reason for hiding this comment

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

(And, allocation-wise, inputs are human-authored so there is a reasonable bound on how much gets allocated here that I'm comfortable with trading for correctness.)

Copy link
Contributor

@dglsparsons dglsparsons Sep 13, 2022

Choose a reason for hiding this comment

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

SGTM then. I guess the input patterns tend to be pretty short too, so it's not like it's an expensive allocation here. 🤔


absoluteFilesToHash, err := globby.GlobFiles(rootPath.ToStringDuringMigration(), prefixedInputPatterns, nil)

if err != nil {
return nil, errors.Wrapf(err, "failed to resolve input globs %v", calculatedInputs)
}
Expand Down
33 changes: 33 additions & 0 deletions cli/internal/hashing/package_deps_hash_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ func requireGitCmd(t *testing.T, repoRoot turbopath.AbsolutePath, args ...string
func TestGetPackageDeps(t *testing.T) {
// Directory structure:
// <root>/
// new-root-file <- new file not added to git
// my-pkg/
// committed-file
// deleted-file
Expand Down Expand Up @@ -280,6 +281,11 @@ func TestGetPackageDeps(t *testing.T) {
err = uncommittedFilePath.WriteFile([]byte("uncommitted bytes"), 0644)
assert.NilError(t, err, "WriteFile")

// create an untracked file in git up a level
rootFilePath := repoRoot.Join("new-root-file")
err = rootFilePath.WriteFile([]byte("new-root bytes"), 0644)
assert.NilError(t, err, "WriteFile")

tests := []struct {
opts *PackageDepsOptions
expected map[turbopath.AnchoredUnixPath]string
Expand Down Expand Up @@ -320,6 +326,20 @@ func TestGetPackageDeps(t *testing.T) {
"dir/nested-file": "bfe53d766e64d78f80050b73cd1c88095bc70abb",
},
},
// inputs with traversal work
{
opts: &PackageDepsOptions{
PackagePath: "my-pkg",
InputPatterns: []string{"../**/*-file"},
},
expected: map[turbopath.AnchoredUnixPath]string{
"../new-root-file": "8906ddcdd634706188bd8ef1c98ac07b9be3425e",
"committed-file": "3a29e62ea9ba15c4a4009d1f605d391cdd262033",
"uncommitted-file": "4e56ad89387e6379e4e91ddfe9872cf6a72c9976",
"package.json": "9e26dfeeb6e641a33dae4961196235bdb965b21b",
"dir/nested-file": "bfe53d766e64d78f80050b73cd1c88095bc70abb",
},
},
// inputs with another glob pattern works
{
opts: &PackageDepsOptions{
Expand All @@ -332,6 +352,19 @@ func TestGetPackageDeps(t *testing.T) {
"uncommitted-file": "4e56ad89387e6379e4e91ddfe9872cf6a72c9976",
},
},
// inputs with another glob pattern + traversal work
{
opts: &PackageDepsOptions{
PackagePath: "my-pkg",
InputPatterns: []string{"../**/{new-root,uncommitted,committed}-file"},
},
expected: map[turbopath.AnchoredUnixPath]string{
"../new-root-file": "8906ddcdd634706188bd8ef1c98ac07b9be3425e",
"committed-file": "3a29e62ea9ba15c4a4009d1f605d391cdd262033",
"package.json": "9e26dfeeb6e641a33dae4961196235bdb965b21b",
"uncommitted-file": "4e56ad89387e6379e4e91ddfe9872cf6a72c9976",
},
},
}
for _, tt := range tests {
got, err := GetPackageDeps(repoRoot, tt.opts)
Expand Down