Skip to content

Commit

Permalink
lib: make git support optional via crate feature
Browse files Browse the repository at this point in the history
I've wanted to make the Git support optional for a long time. However,
since everyone uses the Git backend (and we want to support it even in
the custom binary at Google), there hasn't been much practical reason
to make Git support optional.

Since we now use jj-lib on the server at Google, it does make sense to
have the server not include Git support. In addition to making the
server binary smaller, it would make it easier for us (jj team at
Googlle) to prove that our server is not affected by some libgit2 or
Gitoxide vulnerability. But to be honest, neither of those problems
have come up, so it's more of an excuse to make the Git support
optional at this point.

It turned out to be much simpler than I expected to make Git support
in the lib crate optional. We have done a pretty good job of keeping
Git-related logic separated there.

If we make Git support optional in the lib crate, it's going to make
it a bit harder to move logic from the CLI crate into the lib crate
(as we have planned to do). Maybe that's good, though, since it helps
remind us to keep Git-related logic separated.
  • Loading branch information
martinvonz committed Jun 11, 2024
1 parent 22c5cc0 commit d729d66
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 47 deletions.
14 changes: 14 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,20 @@ jobs:
env:
RUST_BACKTRACE: 1

build-no-git:
name: Build jj-lib without Git support
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29

- name: Install Rust
uses: dtolnay/rust-toolchain@1482605bfc5719782e1267fd0c0cc350fe7646b8
with:
toolchain: 1.76
- name: Build
run: cargo build -p jj-lib --no-default-features --verbose

check-protos:
name: Check protos
runs-on: ubuntu-latest
Expand Down
7 changes: 4 additions & 3 deletions lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ config = { workspace = true }
digest = { workspace = true }
either = { workspace = true }
futures = { workspace = true }
git2 = { workspace = true }
gix = { workspace = true }
git2 = { workspace = true, optional = true }
gix = { workspace = true, optional = true }
glob = { workspace = true }
hex = { workspace = true }
ignore = { workspace = true }
Expand Down Expand Up @@ -91,7 +91,8 @@ testutils = { workspace = true }
tokio = { workspace = true, features = ["full"] }

[features]
default = []
default = ["git"]
git = ["dep:git2", "dep:gix"]
vendored-openssl = ["git2/vendored-openssl"]
watchman = ["dep:tokio", "dep:watchman_client"]
testing = []
2 changes: 2 additions & 0 deletions lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ pub mod fileset;
mod fileset_parser;
pub mod fmt_util;
pub mod fsmonitor;
#[cfg(feature = "git")]
pub mod git;
#[cfg(feature = "git")]
pub mod git_backend;
pub mod gitignore;
pub mod gpg_signing;
Expand Down
10 changes: 7 additions & 3 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ use crate::commit_builder::CommitBuilder;
use crate::default_index::{DefaultIndexStore, DefaultMutableIndex};
use crate::default_submodule_store::DefaultSubmoduleStore;
use crate::file_util::{IoResultExt as _, PathError};
use crate::git_backend::GitBackend;
use crate::index::{ChangeIdIndex, Index, IndexStore, MutableIndex, ReadonlyIndex};
use crate::local_backend::LocalBackend;
use crate::object_id::{HexPrefix, ObjectId, PrefixResolution};
Expand Down Expand Up @@ -378,9 +377,14 @@ impl Default for StoreFactories {
LocalBackend::name(),
Box::new(|_settings, store_path| Ok(Box::new(LocalBackend::load(store_path)))),
);
#[cfg(feature = "git")]
factories.add_backend(
GitBackend::name(),
Box::new(|settings, store_path| Ok(Box::new(GitBackend::load(settings, store_path)?))),
crate::git_backend::GitBackend::name(),
Box::new(|settings, store_path| {
Ok(Box::new(crate::git_backend::GitBackend::load(
settings, store_path,
)?))
}),
);
#[cfg(feature = "testing")]
factories.add_backend(
Expand Down
17 changes: 14 additions & 3 deletions lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub use crate::revset_parser::{
};
use crate::store::Store;
use crate::str_util::StringPattern;
use crate::{dsl_util, git, revset_parser};
use crate::{dsl_util, revset_parser};

/// Error occurred during symbol resolution.
#[derive(Debug, Error)]
Expand Down Expand Up @@ -1313,7 +1313,8 @@ pub fn walk_revs<'index>(
fn resolve_remote_branch(repo: &dyn Repo, name: &str, remote: &str) -> Option<Vec<CommitId>> {
let view = repo.view();
let target = match (name, remote) {
("HEAD", git::REMOTE_NAME_FOR_LOCAL_GIT_REPO) => view.git_head(),
#[cfg(feature = "git")]
("HEAD", crate::git::REMOTE_NAME_FOR_LOCAL_GIT_REPO) => view.git_head(),
(name, remote) => &view.get_remote_branch(name, remote).target,
};
target
Expand Down Expand Up @@ -1605,7 +1606,17 @@ fn resolve_commit_ref(
let commit_ids = repo
.view()
.remote_branches_matching(branch_pattern, remote_pattern)
.filter(|&((_, remote_name), _)| remote_name != git::REMOTE_NAME_FOR_LOCAL_GIT_REPO)
.filter(|&((_, remote_name), _)| {
#[cfg(feature = "git")]
{
remote_name != crate::git::REMOTE_NAME_FOR_LOCAL_GIT_REPO
}
#[cfg(not(feature = "git"))]
{
let _ = remote_name;
true
}
})
.flat_map(|(_, remote_ref)| remote_ref.target.added_ids())
.cloned()
.collect();
Expand Down
27 changes: 19 additions & 8 deletions lib/src/simple_op_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use crate::op_store::{
OpStore, OpStoreError, OpStoreResult, Operation, OperationId, OperationMetadata, RefTarget,
RemoteRef, RemoteRefState, RemoteView, View, ViewId, WorkspaceId,
};
use crate::{dag_walk, git, op_store};
use crate::{dag_walk, op_store};

// BLAKE2b-512 hash length in bytes
const OPERATION_ID_LENGTH: usize = 64;
Expand Down Expand Up @@ -528,8 +528,11 @@ fn branch_views_from_proto_legacy(
// than deleted but yet-to-be-pushed local branch. Alternatively, we could read
// git.auto-local-branch setting here, but that wouldn't always work since the
// setting could be toggled after the branch got merged.
#[cfg(feature = "git")]
let is_git_tracking =
remote_branch.remote_name == git::REMOTE_NAME_FOR_LOCAL_GIT_REPO;
remote_branch.remote_name == crate::git::REMOTE_NAME_FOR_LOCAL_GIT_REPO;
#[cfg(not(feature = "git"))]
let is_git_tracking = false;
let default_state = if is_git_tracking || local_target.is_present() {
RemoteRefState::Tracking
} else {
Expand Down Expand Up @@ -578,13 +581,21 @@ fn migrate_git_refs_to_remote(view: &mut View) {
git_view.branches.insert(name.to_owned(), remote_ref);
}
}
view.remote_views
.insert(git::REMOTE_NAME_FOR_LOCAL_GIT_REPO.to_owned(), git_view);
#[cfg(feature = "git")]
{
view.remote_views.insert(
crate::git::REMOTE_NAME_FOR_LOCAL_GIT_REPO.to_owned(),
git_view,
);

// jj < 0.9 might have imported refs from remote named "git"
let reserved_git_ref_prefix = format!("refs/remotes/{}/", git::REMOTE_NAME_FOR_LOCAL_GIT_REPO);
view.git_refs
.retain(|name, _| !name.starts_with(&reserved_git_ref_prefix));
// jj < 0.9 might have imported refs from remote named "git"
let reserved_git_ref_prefix = format!(
"refs/remotes/{}/",
crate::git::REMOTE_NAME_FOR_LOCAL_GIT_REPO
);
view.git_refs
.retain(|name, _| !name.starts_with(&reserved_git_ref_prefix));
}
}

fn ref_target_to_proto(value: &RefTarget) -> Option<crate::protos::op_store::RefTarget> {
Expand Down
72 changes: 42 additions & 30 deletions lib/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ use std::sync::Arc;

use thiserror::Error;

use crate::backend::{Backend, BackendInitError, MergedTreeId};
use crate::backend::{BackendInitError, MergedTreeId};
use crate::commit::Commit;
use crate::file_util::{self, IoResultExt as _, PathError};
use crate::git_backend::{canonicalize_git_repo_path, GitBackend};
use crate::file_util::{IoResultExt as _, PathError};
use crate::local_backend::LocalBackend;
use crate::local_working_copy::{LocalWorkingCopy, LocalWorkingCopyFactory};
use crate::op_store::{OperationId, WorkspaceId};
Expand Down Expand Up @@ -157,36 +156,44 @@ impl Workspace {

/// Initializes a workspace with a new Git backend and bare Git repo in
/// `.jj/repo/store/git`.
#[cfg(feature = "git")]
pub fn init_internal_git(
user_settings: &UserSettings,
workspace_root: &Path,
) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
let backend_initializer: &BackendInitializer =
&|settings, store_path| Ok(Box::new(GitBackend::init_internal(settings, store_path)?));
let backend_initializer: &BackendInitializer = &|settings, store_path| {
Ok(Box::new(crate::git_backend::GitBackend::init_internal(
settings, store_path,
)?))
};
let signer = Signer::from_settings(user_settings)?;
Self::init_with_backend(user_settings, workspace_root, backend_initializer, signer)
}

/// Initializes a workspace with a new Git backend and Git repo that shares
/// the same working copy.
#[cfg(feature = "git")]
pub fn init_colocated_git(
user_settings: &UserSettings,
workspace_root: &Path,
) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
let backend_initializer = |settings: &UserSettings,
store_path: &Path|
-> Result<Box<dyn Backend>, _> {
-> Result<Box<dyn crate::backend::Backend>, _> {
// TODO: Clean up path normalization. store_path is canonicalized by
// ReadonlyRepo::init(). workspace_root will be canonicalized by
// Workspace::new(), but it's not yet here.
let store_relative_workspace_root =
if let Ok(workspace_root) = workspace_root.canonicalize() {
file_util::relative_path(store_path, &workspace_root)
crate::file_util::relative_path(store_path, &workspace_root)
} else {
workspace_root.to_owned()
};
let backend =
GitBackend::init_colocated(settings, store_path, &store_relative_workspace_root)?;
let backend = crate::git_backend::GitBackend::init_colocated(
settings,
store_path,
&store_relative_workspace_root,
)?;
Ok(Box::new(backend))
};
let signer = Signer::from_settings(user_settings)?;
Expand All @@ -197,33 +204,38 @@ impl Workspace {
///
/// The `git_repo_path` usually ends with `.git`. It's the path to the Git
/// repo directory, not the working directory.
#[cfg(feature = "git")]
pub fn init_external_git(
user_settings: &UserSettings,
workspace_root: &Path,
git_repo_path: &Path,
) -> Result<(Self, Arc<ReadonlyRepo>), WorkspaceInitError> {
let backend_initializer =
|settings: &UserSettings, store_path: &Path| -> Result<Box<dyn Backend>, _> {
// If the git repo is inside the workspace, use a relative path to it so the
// whole workspace can be moved without breaking.
// TODO: Clean up path normalization. store_path is canonicalized by
// ReadonlyRepo::init(). workspace_root will be canonicalized by
// Workspace::new(), but it's not yet here.
let store_relative_git_repo_path = match (
workspace_root.canonicalize(),
canonicalize_git_repo_path(git_repo_path),
) {
(Ok(workspace_root), Ok(git_repo_path))
if git_repo_path.starts_with(&workspace_root) =>
{
file_util::relative_path(store_path, &git_repo_path)
}
_ => git_repo_path.to_owned(),
};
let backend =
GitBackend::init_external(settings, store_path, &store_relative_git_repo_path)?;
Ok(Box::new(backend))
let backend_initializer = |settings: &UserSettings,
store_path: &Path|
-> Result<Box<dyn crate::backend::Backend>, _> {
// If the git repo is inside the workspace, use a relative path to it so the
// whole workspace can be moved without breaking.
// TODO: Clean up path normalization. store_path is canonicalized by
// ReadonlyRepo::init(). workspace_root will be canonicalized by
// Workspace::new(), but it's not yet here.
let store_relative_git_repo_path = match (
workspace_root.canonicalize(),
crate::git_backend::canonicalize_git_repo_path(git_repo_path),
) {
(Ok(workspace_root), Ok(git_repo_path))
if git_repo_path.starts_with(&workspace_root) =>
{
crate::file_util::relative_path(store_path, &git_repo_path)
}
_ => git_repo_path.to_owned(),
};
let backend = crate::git_backend::GitBackend::init_external(
settings,
store_path,
&store_relative_git_repo_path,
)?;
Ok(Box::new(backend))
};
let signer = Signer::from_settings(user_settings)?;
Self::init_with_backend(user_settings, workspace_root, &backend_initializer, signer)
}
Expand Down

0 comments on commit d729d66

Please sign in to comment.