-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix(turborepo): Use which crate to resolve yarn binary #5001
Changes from 5 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,3 +197,4 @@ tracing-subscriber = "0.3.16" | |
url = "2.2.2" | ||
urlencoding = "2.1.2" | ||
webbrowser = "0.8.7" | ||
which = "4.4.0" | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ use std::{ | |
use turbopath::{ | ||
AbsoluteSystemPath, AbsoluteSystemPathBuf, AnchoredSystemPathBuf, RelativeUnixPath, | ||
}; | ||
use which::which; | ||
|
||
use crate::Error; | ||
|
||
|
@@ -76,7 +77,8 @@ fn execute_git_command( | |
args: &[&str], | ||
pathspec: &str, | ||
) -> Result<Vec<u8>, Error> { | ||
let mut command = Command::new("git"); | ||
let git_binary = which("git")?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're going to use |
||
let mut command = Command::new(git_binary); | ||
command.args(args).current_dir(git_root); | ||
|
||
add_pathspec(&mut command, pathspec); | ||
|
@@ -156,7 +158,8 @@ pub fn previous_content( | |
file_path.as_path().try_into()? | ||
}; | ||
|
||
let mut command = Command::new("git"); | ||
let git_binary = which("git")?; | ||
let mut command = Command::new(git_binary); | ||
let command = command | ||
.arg("show") | ||
.arg(format!( | ||
|
@@ -190,6 +193,7 @@ mod tests { | |
use git2::{Oid, Repository}; | ||
use tempfile::TempDir; | ||
use turbopath::{PathError, PathValidationError}; | ||
use which::which; | ||
|
||
use super::previous_content; | ||
use crate::{git::changed_files, Error}; | ||
|
@@ -253,7 +257,9 @@ mod tests { | |
#[test] | ||
fn test_shallow_clone() -> Result<(), Error> { | ||
let tmp_dir = tempfile::tempdir()?; | ||
let output = Command::new("git") | ||
|
||
let git_binary = which("git")?; | ||
let output = Command::new(git_binary) | ||
.args(&[ | ||
"clone", | ||
"--depth", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,3 +36,48 @@ Set package manager to pnpm in package.json | |
Run test run | ||
$ ${TURBO} run build --__test-run | jq .package_manager | ||
"pnpm" | ||
|
||
Clear package manager field in package.json | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to verify this manually on windows, these tests don't run there |
||
$ jq 'del(.packageManager)' package.json > package.json.tmp && mv package.json.tmp package.json | ||
|
||
Delete package-lock.json | ||
$ rm package-lock.json | ||
|
||
Use yarn 1.22.19 | ||
$ corepack prepare yarn@1.22.19 --activate | ||
Preparing yarn@1.22.19 for immediate activation... | ||
|
||
Create yarn.lock | ||
$ touch yarn.lock | ||
|
||
Run test run | ||
$ ${TURBO} run build --__test-run | jq .package_manager | ||
"yarn" | ||
|
||
Use yarn 3.5.1 | ||
$ corepack prepare yarn@3.5.1 --activate | ||
Preparing yarn@3.5.1 for immediate activation... | ||
|
||
Run test run | ||
$ ${TURBO} run build --__test-run | jq .package_manager | ||
"berry" | ||
|
||
Delete yarn.lock | ||
$ rm yarn.lock | ||
|
||
Create pnpm-lock.yaml | ||
$ touch pnpm-lock.yaml | ||
|
||
Run test run | ||
$ ${TURBO} run build --__test-run | jq .package_manager | ||
"pnpm" | ||
|
||
Delete pnpm-lock.yaml | ||
$ rm pnpm-lock.yaml | ||
|
||
Create package-lock.json | ||
$ touch package-lock.json | ||
|
||
Run test run | ||
$ ${TURBO} run build --__test-run | jq .package_manager | ||
"npm" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the lack of a newline here is hitting the linter