Skip to content

Commit

Permalink
make RepoConfig and CommandBase use absolute system paths (#4693)
Browse files Browse the repository at this point in the history
### Description

Continue the correctness work by making the base command provide an
AbsoluteSystemPathBuf
  • Loading branch information
arlyon committed May 2, 2023
1 parent 2c7bad5 commit 0a7661b
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 32 deletions.
4 changes: 4 additions & 0 deletions crates/turborepo-lib/src/cli.rs
Expand Up @@ -11,6 +11,7 @@ use clap_complete::{generate, Shell};
use dunce::canonicalize as fs_canonicalize;
use serde::Serialize;
use tracing::{debug, error};
use turbopath::AbsoluteSystemPathBuf;

use crate::{
commands::{bin, daemon, link, login, logout, unlink, CommandBase},
Expand Down Expand Up @@ -489,6 +490,9 @@ pub async fn run(repo_state: Option<RepoState>) -> Result<Payload> {
current_dir()?
};

// a non-absolute repo root is a bug
let repo_root = AbsoluteSystemPathBuf::new(repo_root).expect("repo_root is not absolute");

let version = get_version();

match cli_args.command.as_ref().unwrap() {
Expand Down
30 changes: 22 additions & 8 deletions crates/turborepo-lib/src/commands/link.rs
Expand Up @@ -16,6 +16,7 @@ use dialoguer::{theme::ColorfulTheme, Confirm};
use dirs_next::home_dir;
#[cfg(test)]
use rand::Rng;
use turbopath::RelativeSystemPathBuf;
use turborepo_api_client::{APIClient, CachingStatus, Space, Team};

#[cfg(not(test))]
Expand Down Expand Up @@ -166,8 +167,11 @@ pub async fn link(
verify_caching_enabled(&api_client, team_id, token, Some(selected_team.clone()))
.await?;

fs::create_dir_all(base.repo_root.join(".turbo"))
.context("could not create .turbo directory")?;
fs::create_dir_all(
base.repo_root
.join_relative(RelativeSystemPathBuf::new(".turbo").expect("relative")),
)
.context("could not create .turbo directory")?;
base.repo_config_mut()?
.set_team_id(Some(team_id.to_string()))?;

Expand Down Expand Up @@ -395,7 +399,9 @@ fn enable_caching(url: &str) -> Result<()> {
}

fn add_turbo_to_gitignore(base: &CommandBase) -> Result<()> {
let gitignore_path = base.repo_root.join(".gitignore");
let gitignore_path = base
.repo_root
.join_relative(RelativeSystemPathBuf::new(".gitignore").expect("relative"));

if !gitignore_path.exists() {
let mut gitignore = File::create(gitignore_path)?;
Expand All @@ -419,7 +425,9 @@ fn add_turbo_to_gitignore(base: &CommandBase) -> Result<()> {
}

fn add_space_id_to_turbo_json(base: &CommandBase, space_id: &str) -> Result<()> {
let turbo_json_path = base.repo_root.join("turbo.json");
let turbo_json_path = base
.repo_root
.join_relative(RelativeSystemPathBuf::new("turbo.json").expect("relative"));

if !turbo_json_path.exists() {
return Err(anyhow!("turbo.json not found."));
Expand Down Expand Up @@ -453,6 +461,7 @@ mod test {

use tempfile::{NamedTempFile, TempDir};
use tokio::sync::OnceCell;
use turbopath::{AbsoluteSystemPathBuf, RelativeSystemPathBuf};
use vercel_api_mock::start_test_server;

use crate::{
Expand All @@ -468,6 +477,7 @@ mod test {
let user_config_file = NamedTempFile::new().unwrap();
fs::write(user_config_file.path(), r#"{ "token": "hello" }"#).unwrap();
let repo_config_file = NamedTempFile::new().unwrap();
let repo_config_path = AbsoluteSystemPathBuf::new(repo_config_file.path()).unwrap();
fs::write(
repo_config_file.path(),
r#"{ "apiurl": "http://localhost:3000" }"#,
Expand All @@ -487,7 +497,7 @@ mod test {
.unwrap(),
),
repo_config: OnceCell::from(
RepoConfigLoader::new(repo_config_file.path().to_path_buf())
RepoConfigLoader::new(repo_config_path)
.with_api(Some(format!("http://localhost:{}", port)))
.with_login(Some(format!("http://localhost:{}", port)))
.load()
Expand Down Expand Up @@ -517,6 +527,7 @@ mod test {

// repo config
let repo_config_file = NamedTempFile::new().unwrap();
let repo_config_path = AbsoluteSystemPathBuf::new(repo_config_file.path()).unwrap();
fs::write(
repo_config_file.path(),
r#"{ "apiurl": "http://localhost:3000" }"#,
Expand All @@ -526,7 +537,7 @@ mod test {
let port = port_scanner::request_open_port().unwrap();
let handle = tokio::spawn(start_test_server(port));
let mut base = CommandBase {
repo_root: TempDir::new().unwrap().into_path(),
repo_root: AbsoluteSystemPathBuf::new(TempDir::new().unwrap().into_path()).unwrap(),
ui: UI::new(false),
client_config: OnceCell::from(ClientConfigLoader::new().load().unwrap()),
user_config: OnceCell::from(
Expand All @@ -536,7 +547,7 @@ mod test {
.unwrap(),
),
repo_config: OnceCell::from(
RepoConfigLoader::new(repo_config_file.path().to_path_buf())
RepoConfigLoader::new(repo_config_path)
.with_api(Some(format!("http://localhost:{}", port)))
.with_login(Some(format!("http://localhost:{}", port)))
.load()
Expand All @@ -547,7 +558,10 @@ mod test {
};

// turbo config
let turbo_json_file = base.repo_root.join("turbo.json");
let turbo_json_file = base
.repo_root
.join_relative(RelativeSystemPathBuf::new("turbo.json").expect("relative"));

fs::write(
turbo_json_file.as_path(),
r#"{ "globalEnv": [], "pipeline": {} }"#,
Expand Down
7 changes: 5 additions & 2 deletions crates/turborepo-lib/src/commands/login.rs
Expand Up @@ -304,6 +304,7 @@ mod test {
use serde::Deserialize;
use tempfile::NamedTempFile;
use tokio::sync::OnceCell;
use turbopath::AbsoluteSystemPathBuf;
use vercel_api_mock::start_test_server;

use crate::{
Expand All @@ -325,6 +326,7 @@ mod test {
let user_config_file = NamedTempFile::new().unwrap();
fs::write(user_config_file.path(), r#"{ "token": "hello" }"#).unwrap();
let repo_config_file = NamedTempFile::new().unwrap();
let repo_config_path = AbsoluteSystemPathBuf::new(repo_config_file.path()).unwrap();
// Explicitly pass the wrong port to confirm that we're reading it from the
// manual override
fs::write(
Expand All @@ -343,7 +345,7 @@ mod test {
.unwrap(),
),
repo_config: OnceCell::from(
RepoConfigLoader::new(repo_config_file.path().to_path_buf())
RepoConfigLoader::new(repo_config_path)
.with_api(Some(format!("http://localhost:{}", port)))
.load()
.unwrap(),
Expand Down Expand Up @@ -376,6 +378,7 @@ mod test {
let user_config_file = NamedTempFile::new().unwrap();
fs::write(user_config_file.path(), r#"{ "token": "hello" }"#).unwrap();
let repo_config_file = NamedTempFile::new().unwrap();
let repo_config_path = AbsoluteSystemPathBuf::new(repo_config_file.path()).unwrap();
// Explicitly pass the wrong port to confirm that we're reading it from the
// manual override
fs::write(
Expand All @@ -394,7 +397,7 @@ mod test {
.unwrap(),
),
repo_config: OnceCell::from(
RepoConfigLoader::new(repo_config_file.path().to_path_buf())
RepoConfigLoader::new(repo_config_path)
.with_api(Some(format!("http://localhost:{}", port)))
.load()
.unwrap(),
Expand Down
32 changes: 25 additions & 7 deletions crates/turborepo-lib/src/commands/mod.rs
@@ -1,8 +1,7 @@
use std::path::PathBuf;

use anyhow::Result;
use sha2::{Digest, Sha256};
use tokio::sync::OnceCell;
use turbopath::AbsoluteSystemPathBuf;
use turborepo_api_client::APIClient;

use crate::{
Expand All @@ -22,7 +21,7 @@ pub(crate) mod logout;
pub(crate) mod unlink;

pub struct CommandBase {
pub repo_root: PathBuf,
pub repo_root: AbsoluteSystemPathBuf,
pub ui: UI,
user_config: OnceCell<UserConfig>,
repo_config: OnceCell<RepoConfig>,
Expand All @@ -32,7 +31,11 @@ pub struct CommandBase {
}

impl CommandBase {
pub fn new(args: Args, repo_root: PathBuf, version: &'static str) -> Result<Self> {
pub fn new(
args: Args,
repo_root: AbsoluteSystemPathBuf,
version: &'static str,
) -> Result<Self> {
Ok(Self {
repo_root,
ui: args.ui(),
Expand Down Expand Up @@ -162,19 +165,34 @@ impl CommandBase {
#[cfg(test)]
mod test {
use test_case::test_case;
use turbopath::AbsoluteSystemPathBuf;

use crate::get_version;

#[cfg(not(target_os = "windows"))]
#[test_case("/tmp/turborepo", "6e0cfa616f75a61c"; "basic example")]
#[test_case("", "e3b0c44298fc1c14"; "empty string ok")]
fn test_repo_hash(path: &str, expected_hash: &str) {
use std::path::PathBuf;
use super::CommandBase;
use crate::Args;

let args = Args::default();
let repo_root = AbsoluteSystemPathBuf::new(path).unwrap();
let command_base = CommandBase::new(args, repo_root, get_version()).unwrap();

let hash = command_base.repo_hash();

assert_eq!(hash, expected_hash);
assert_eq!(hash.len(), 16);
}

#[cfg(target_os = "windows")]
#[test_case("C:\\\\tmp\\turborepo", "0103736e6883e35f"; "basic example")]
fn test_repo_hash_win(path: &str, expected_hash: &str) {
use super::CommandBase;
use crate::Args;

let args = Args::default();
let repo_root = PathBuf::from(path);
let repo_root = AbsoluteSystemPathBuf::new(path).unwrap();
let command_base = CommandBase::new(args, repo_root, get_version()).unwrap();

let hash = command_base.repo_hash();
Expand Down
5 changes: 4 additions & 1 deletion crates/turborepo-lib/src/commands/unlink.rs
@@ -1,6 +1,7 @@
use std::fs::File;

use anyhow::{Context, Result};
use turbopath::RelativeSystemPathBuf;

use crate::{cli::LinkTarget, commands::CommandBase, config::TurboJson, ui::GREY};

Expand Down Expand Up @@ -53,7 +54,9 @@ pub fn unlink(base: &mut CommandBase, target: LinkTarget) -> Result<()> {
}

fn remove_spaces_from_turbo_json(base: &CommandBase) -> Result<UnlinkSpacesResult> {
let turbo_json_path = base.repo_root.join("turbo.json");
let turbo_json_path = base
.repo_root
.join_relative(RelativeSystemPathBuf::new("turbo.json").expect("relative"));

let turbo_json_file = File::open(&turbo_json_path).context("unable to open turbo.json file")?;
let mut turbo_json: TurboJson = serde_json::from_reader(turbo_json_file)?;
Expand Down
44 changes: 30 additions & 14 deletions crates/turborepo-lib/src/config/repo.rs
Expand Up @@ -7,6 +7,7 @@ use std::{
use anyhow::Result;
use config::Config;
use serde::{Deserialize, Serialize};
use turbopath::{AbsoluteSystemPathBuf, RelativeSystemPathBuf};

use super::{write_to_disk, MappedEnvironment};

Expand All @@ -17,7 +18,7 @@ const DEFAULT_LOGIN_URL: &str = "https://vercel.com";
pub struct RepoConfig {
disk_config: RepoConfigValue,
config: RepoConfigValue,
path: PathBuf,
path: AbsoluteSystemPathBuf,
}

#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq, Default)]
Expand All @@ -34,7 +35,7 @@ struct RepoConfigValue {

#[derive(Debug, Clone)]
pub struct RepoConfigLoader {
path: PathBuf,
path: AbsoluteSystemPathBuf,
api: Option<String>,
login: Option<String>,
team_slug: Option<String>,
Expand Down Expand Up @@ -77,18 +78,18 @@ impl RepoConfig {
}

fn write_to_disk(&self) -> Result<()> {
write_to_disk(&self.path, &self.disk_config)
write_to_disk(&self.path.as_path(), &self.disk_config)
}
}

#[allow(dead_code)]
pub fn get_repo_config_path(repo_root: &Path) -> PathBuf {
repo_root.join(".turbo").join("config.json")
pub fn get_repo_config_path(repo_root: &AbsoluteSystemPathBuf) -> AbsoluteSystemPathBuf {
let config = RelativeSystemPathBuf::new(".turbo/config.json").expect("is relative");
repo_root.join_relative(config)
}

impl RepoConfigLoader {
#[allow(dead_code)]
pub fn new(path: PathBuf) -> Self {
pub fn new(path: AbsoluteSystemPathBuf) -> Self {
Self {
path,
api: None,
Expand Down Expand Up @@ -188,7 +189,13 @@ mod test {

#[test]
fn test_repo_config_when_missing() -> Result<()> {
let config = RepoConfigLoader::new(PathBuf::from("missing")).load();
let path = if cfg!(windows) {
"C:\\missing"
} else {
"/missing"
};

let config = RepoConfigLoader::new(AbsoluteSystemPathBuf::new(path).unwrap()).load();
assert!(config.is_ok());

Ok(())
Expand All @@ -197,9 +204,10 @@ mod test {
#[test]
fn test_repo_config_with_team_and_api_flags() -> Result<()> {
let mut config_file = NamedTempFile::new()?;
let config_path = AbsoluteSystemPathBuf::new(config_file.path())?;
writeln!(&mut config_file, "{{\"teamId\": \"123\"}}")?;

let config = RepoConfigLoader::new(config_file.path().to_path_buf())
let config = RepoConfigLoader::new(config_path)
.with_team_slug(Some("my-team-slug".into()))
.with_api(Some("http://my-login-url".into()))
.load()?;
Expand All @@ -213,7 +221,13 @@ mod test {

#[test]
fn test_repo_config_includes_defaults() {
let config = RepoConfigLoader::new(PathBuf::from("missing"))
let path = if cfg!(windows) {
"C:\\missing"
} else {
"/missing"
};

let config = RepoConfigLoader::new(AbsoluteSystemPathBuf::new(path).unwrap())
.load()
.unwrap();
assert_eq!(config.api_url(), DEFAULT_API_URL);
Expand All @@ -225,9 +239,9 @@ mod test {
#[test]
fn test_team_override_clears_id() -> Result<()> {
let mut config_file = NamedTempFile::new()?;
let config_path = AbsoluteSystemPathBuf::new(config_file.path())?;
writeln!(&mut config_file, "{{\"teamId\": \"123\"}}")?;
let loader = RepoConfigLoader::new(config_file.path().to_path_buf())
.with_team_slug(Some("foo".into()));
let loader = RepoConfigLoader::new(config_path).with_team_slug(Some("foo".into()));

let config = loader.load()?;
assert_eq!(config.team_slug(), Some("foo"));
Expand All @@ -239,10 +253,11 @@ mod test {
#[test]
fn test_set_team_clears_id() -> Result<()> {
let mut config_file = NamedTempFile::new()?;
let config_path = AbsoluteSystemPathBuf::new(config_file.path())?;
// We will never pragmatically write the "teamslug" field as camelCase,
// but viper is case insensitive and we want to keep this functionality.
writeln!(&mut config_file, "{{\"teamSlug\": \"my-team\"}}")?;
let loader = RepoConfigLoader::new(config_file.path().to_path_buf());
let loader = RepoConfigLoader::new(config_path);

let mut config = loader.clone().load()?;
config.set_team_id(Some("my-team-id".into()))?;
Expand All @@ -257,12 +272,13 @@ mod test {
#[test]
fn test_repo_env_variable() -> Result<()> {
let mut config_file = NamedTempFile::new()?;
let config_path = AbsoluteSystemPathBuf::new(config_file.path())?;
writeln!(&mut config_file, "{{\"teamslug\": \"other-team\"}}")?;
let login_url = "http://my-login-url";
let api_url = "http://my-api";
let team_id = "123";
let team_slug = "my-team";
let config = RepoConfigLoader::new(config_file.path().to_path_buf())
let config = RepoConfigLoader::new(config_path)
.with_environment({
let mut env = HashMap::new();
env.insert("TURBO_API".into(), api_url.into());
Expand Down

0 comments on commit 0a7661b

Please sign in to comment.