Skip to content

Commit

Permalink
working_copy: let caller pass in base Git ignores (#65, #87)
Browse files Browse the repository at this point in the history
The library crate shouldn't look up the user's `$HOME` directory
(maybe the library is used by a server process), so let's have the
caller pass it into the library crate instead.
  • Loading branch information
martinvonz committed Mar 12, 2022
1 parent 7f3ee6c commit 92329b2
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 31 deletions.
29 changes: 15 additions & 14 deletions lib/src/working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,18 +293,12 @@ impl TreeState {

// Look for changes to the working copy. If there are any changes, create
// a new tree from it and return it, and also update the dirstate on disk.
pub fn write_tree(&mut self) -> TreeId {
// TODO: We should probably have the caller pass in the home directory to the
// library crate instead of depending on $HOME directly here. We should also
// have the caller (within the library crate) chain that the
// .jj/git/info/exclude file if we're inside a git-backed repo.
let mut git_ignore = GitIgnoreFile::empty();
if let Ok(home_dir) = std::env::var("HOME") {
let home_dir_path = PathBuf::from(home_dir);
git_ignore = git_ignore.chain_with_file("", home_dir_path.join(".gitignore"));
}

let mut work = vec![(RepoPath::root(), self.working_copy_path.clone(), git_ignore)];
pub fn write_tree(&mut self, base_ignores: Arc<GitIgnoreFile>) -> TreeId {
let mut work = vec![(
RepoPath::root(),
self.working_copy_path.clone(),
base_ignores,
)];
let mut tree_builder = self.store.tree_builder(self.tree_id.clone());
let mut deleted_files: HashSet<_> = self.file_states.keys().cloned().collect();
while !work.is_empty() {
Expand Down Expand Up @@ -869,8 +863,15 @@ impl LockedWorkingCopy<'_> {
&self.old_tree_id
}

pub fn write_tree(&mut self) -> TreeId {
self.wc.tree_state().as_mut().unwrap().write_tree()
// The base_ignores are passed in here rather than being set on the TreeState
// because the TreeState may be long-lived if the library is used in a
// long-lived process.
pub fn write_tree(&mut self, base_ignores: Arc<GitIgnoreFile>) -> TreeId {
self.wc
.tree_state()
.as_mut()
.unwrap()
.write_tree(base_ignores)
}

pub fn check_out(&mut self, new_tree: &Tree) -> Result<CheckoutStats, CheckoutError> {
Expand Down
25 changes: 13 additions & 12 deletions lib/tests/test_working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::sync::Arc;

use itertools::Itertools;
use jujutsu_lib::backend::{Conflict, ConflictPart, TreeValue};
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::op_store::WorkspaceId;
use jujutsu_lib::repo::ReadonlyRepo;
use jujutsu_lib::repo_path::{RepoPath, RepoPathComponent};
Expand All @@ -39,7 +40,7 @@ fn test_root(use_git: bool) {

let wc = test_workspace.workspace.working_copy_mut();
let mut locked_wc = wc.start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
locked_wc.discard();
let checkout_id = repo.view().get_checkout(&WorkspaceId::default()).unwrap();
let checkout_commit = repo.store().get_commit(checkout_id).unwrap();
Expand Down Expand Up @@ -215,7 +216,7 @@ fn test_checkout_file_transitions(use_git: bool) {

// Check that the working copy is clean.
let mut locked_wc = wc.start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
locked_wc.discard();
assert_eq!(new_tree_id, right_tree_id);

Expand Down Expand Up @@ -317,7 +318,7 @@ fn test_reset() {
assert!(ignored_path.to_fs_path(&workspace_root).is_file());
assert!(!wc.file_states().contains_key(&ignored_path));
let mut locked_wc = wc.start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
assert_eq!(new_tree_id, *tree_without_file.id());
locked_wc.discard();

Expand All @@ -330,7 +331,7 @@ fn test_reset() {
assert!(ignored_path.to_fs_path(&workspace_root).is_file());
assert!(!wc.file_states().contains_key(&ignored_path));
let mut locked_wc = wc.start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
assert_eq!(new_tree_id, *tree_without_file.id());
locked_wc.discard();

Expand All @@ -342,7 +343,7 @@ fn test_reset() {
assert!(ignored_path.to_fs_path(&workspace_root).is_file());
assert!(wc.file_states().contains_key(&ignored_path));
let mut locked_wc = wc.start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
assert_eq!(new_tree_id, *tree_with_file.id());
locked_wc.discard();
}
Expand Down Expand Up @@ -418,7 +419,7 @@ fn test_commit_racy_timestamps(use_git: bool) {
.unwrap();
}
let mut locked_wc = wc.start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
locked_wc.discard();
assert_ne!(new_tree_id, previous_tree_id);
previous_tree_id = new_tree_id;
Expand Down Expand Up @@ -452,7 +453,7 @@ fn test_gitignores(use_git: bool) {

let wc = test_workspace.workspace.working_copy_mut();
let mut locked_wc = wc.start_mutation();
let new_tree_id1 = locked_wc.write_tree();
let new_tree_id1 = locked_wc.write_tree(GitIgnoreFile::empty());
locked_wc.finish(repo.op_id().clone());
let tree1 = repo
.store()
Expand Down Expand Up @@ -482,7 +483,7 @@ fn test_gitignores(use_git: bool) {
testutils::write_working_copy_file(&workspace_root, &subdir_ignored_path, "2");

let mut locked_wc = wc.start_mutation();
let new_tree_id2 = locked_wc.write_tree();
let new_tree_id2 = locked_wc.write_tree(GitIgnoreFile::empty());
locked_wc.discard();
let tree2 = repo
.store()
Expand Down Expand Up @@ -542,7 +543,7 @@ fn test_gitignores_checkout_overwrites_ignored(use_git: bool) {

// Check that the file is in the tree created by committing the working copy
let mut locked_wc = wc.start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
locked_wc.discard();
let new_tree = repo
.store()
Expand Down Expand Up @@ -588,7 +589,7 @@ fn test_gitignores_ignored_directory_already_tracked(use_git: bool) {
// Check that the file is still in the tree created by committing the working
// copy (that it didn't get removed because the directory is ignored)
let mut locked_wc = wc.start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
locked_wc.discard();
let new_tree = repo
.store()
Expand Down Expand Up @@ -619,7 +620,7 @@ fn test_dotgit_ignored(use_git: bool) {
"contents",
);
let mut locked_wc = test_workspace.workspace.working_copy_mut().start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
assert_eq!(new_tree_id, *repo.store().empty_tree_id());
locked_wc.discard();
std::fs::remove_dir_all(&dotgit_path).unwrap();
Expand All @@ -631,7 +632,7 @@ fn test_dotgit_ignored(use_git: bool) {
"contents",
);
let mut locked_wc = test_workspace.workspace.working_copy_mut().start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
assert_eq!(new_tree_id, *repo.store().empty_tree_id());
locked_wc.discard();
}
3 changes: 2 additions & 1 deletion lib/tests/test_working_copy_concurrent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use std::cmp::max;
use std::thread;

use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::testutils;
use jujutsu_lib::working_copy::CheckoutError;
Expand Down Expand Up @@ -128,7 +129,7 @@ fn test_checkout_parallel(use_git: bool) {
// write_tree() should take the same lock as check_out(), write_tree()
// should never produce a different tree.
let mut locked_wc = workspace.working_copy_mut().start_mutation();
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(GitIgnoreFile::empty());
locked_wc.discard();
assert!(tree_ids.contains(&new_tree_id));
});
Expand Down
28 changes: 25 additions & 3 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use jujutsu_lib::dag_walk::topo_order_reverse;
use jujutsu_lib::diff::{Diff, DiffHunk};
use jujutsu_lib::files::DiffLine;
use jujutsu_lib::git::{GitExportError, GitFetchError, GitImportError, GitRefUpdate};
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::index::HexPrefix;
use jujutsu_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher};
use jujutsu_lib::op_heads_store::OpHeadsStore;
Expand Down Expand Up @@ -381,6 +382,19 @@ impl WorkspaceCommandHelper {
self.working_copy_shared_with_git
}

fn base_ignores(&self) -> Arc<GitIgnoreFile> {
let mut git_ignores = GitIgnoreFile::empty();
if let Ok(home_dir) = std::env::var("HOME") {
let home_dir_path = PathBuf::from(home_dir);
// TODO: Look up the name of the file in the core.excludesFile config instead of
// hard-coding its name like this.
git_ignores = git_ignores.chain_with_file("", home_dir_path.join(".gitignore"));
}
// TODO: Chain with the .jj/git/info/exclude file if we're inside a git-backed
// repo.
git_ignores
}

fn resolve_revision_arg(
&mut self,
ui: &mut Ui,
Expand Down Expand Up @@ -498,6 +512,7 @@ impl WorkspaceCommandHelper {
return Ok(());
}
};
let base_ignores = self.base_ignores();
let mut locked_wc = self.workspace.working_copy_mut().start_mutation();
// Check if the working copy commit matches the repo's view. It's fine if it
// doesn't, but we'll need to reload the repo so the new commit is
Expand Down Expand Up @@ -557,7 +572,7 @@ impl WorkspaceCommandHelper {
)));
}
}
let new_tree_id = locked_wc.write_tree();
let new_tree_id = locked_wc.write_tree(base_ignores);
if new_tree_id != *checkout_commit.tree_id() {
let mut tx = self.repo.start_transaction("commit working copy");
let mut_repo = tx.mut_repo();
Expand Down Expand Up @@ -595,7 +610,13 @@ impl WorkspaceCommandHelper {
right_tree: &Tree,
instructions: &str,
) -> Result<TreeId, DiffEditError> {
crate::diff_edit::edit_diff(&self.settings, left_tree, right_tree, instructions)
crate::diff_edit::edit_diff(
&self.settings,
left_tree,
right_tree,
instructions,
self.base_ignores(),
)
}

fn start_transaction(&self, description: &str) -> Transaction {
Expand Down Expand Up @@ -1839,6 +1860,7 @@ fn cmd_untrack(
};

let mut tx = workspace_command.start_transaction("untrack paths");
let base_ignores = workspace_command.base_ignores();
let mut locked_working_copy = workspace_command.working_copy_mut().start_mutation();
if current_checkout.tree_id() != locked_working_copy.old_tree_id() {
return Err(CommandError::UserError(
Expand All @@ -1856,7 +1878,7 @@ fn cmd_untrack(
locked_working_copy.reset(&new_tree)?;
// Commit the working copy again so we can inform the user if paths couldn't be
// untracked because they're not ignored.
let wc_tree_id = locked_working_copy.write_tree();
let wc_tree_id = locked_working_copy.write_tree(base_ignores);
if wc_tree_id != new_tree_id {
let wc_tree = store.get_tree(&RepoPath::root(), &wc_tree_id)?;
let added_back = wc_tree.entries_matching(matcher.as_ref()).collect_vec();
Expand Down
4 changes: 3 additions & 1 deletion src/diff_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::process::Command;
use std::sync::Arc;

use jujutsu_lib::backend::{BackendError, TreeId};
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::settings::UserSettings;
Expand Down Expand Up @@ -79,6 +80,7 @@ pub fn edit_diff(
left_tree: &Tree,
right_tree: &Tree,
instructions: &str,
base_ignores: Arc<GitIgnoreFile>,
) -> Result<TreeId, DiffEditError> {
// First create partial Trees of only the subset of the left and right trees
// that affect files changed between them.
Expand Down Expand Up @@ -148,7 +150,7 @@ pub fn edit_diff(

// Create a Tree based on the initial right tree, applying the changes made to
// that directory by the diff editor.
let new_right_partial_tree_id = right_tree_state.write_tree();
let new_right_partial_tree_id = right_tree_state.write_tree(base_ignores);
let new_right_partial_tree = store.get_tree(&RepoPath::root(), &new_right_partial_tree_id)?;
let new_tree_id = merge_trees(right_tree, &right_partial_tree, &new_right_partial_tree)?;

Expand Down

0 comments on commit 92329b2

Please sign in to comment.