scs_server: add file_diff method

Summary:
Add a method to diff file contents directly, even if their paths are not known.

This provides a headerless diff, just of the content differences.

Reviewed By: ahornby

Differential Revision: D24200803

fbshipit-source-id: 32cf461d0c43a7b785ae58bec284978ff7976702
This commit is contained in:
Mark Thomas 2020-10-20 05:43:05 -07:00 committed by Facebook GitHub Bot
parent 8945452492
commit 7c8ee4f52f
9 changed files with 165 additions and 4 deletions

View File

@ -18,6 +18,7 @@ use filestore::{self, get_metadata, FetchKey};
use futures::compat::{Future01CompatExt, Stream01CompatExt};
use futures::future::{FutureExt, Shared};
use futures::stream::TryStreamExt;
use futures::try_join;
use crate::errors::MononokeError;
use crate::repo::RepoContext;
@ -193,6 +194,36 @@ impl FileContext {
}
}
/// A diff between two files in headerless unified diff format
pub struct HeaderlessUnifiedDiff {
/// Raw diff as bytes.
pub raw_diff: Vec<u8>,
/// One of the diffed files is binary, raw diff contains just a placeholder.
pub is_binary: bool,
}
pub async fn headerless_unified_diff(
old_file: &FileContext,
new_file: &FileContext,
context_lines: usize,
) -> Result<HeaderlessUnifiedDiff, MononokeError> {
let (old_diff_file, new_diff_file) =
try_join!(old_file.content_concat(), new_file.content_concat(),)?;
let is_binary = old_diff_file.contains(&0) || new_diff_file.contains(&0);
let raw_diff = if is_binary {
b"Binary files differ".to_vec()
} else {
let opts = xdiff::HeaderlessDiffOpts {
context: context_lines,
};
xdiff::diff_unified_headerless(&old_diff_file, &new_diff_file, opts)
};
Ok(HeaderlessUnifiedDiff {
raw_diff,
is_binary,
})
}
/// File contexts should only exist for files that are known to be in the
/// blobstore. If attempting to access the content results in an error, this
/// error is returned. This is an internal error, as it means either the data

View File

@ -52,7 +52,9 @@ pub use crate::changeset_path::{
};
pub use crate::changeset_path_diff::ChangesetPathDiffContext;
pub use crate::errors::MononokeError;
pub use crate::file::{FileContext, FileId, FileMetadata, FileType};
pub use crate::file::{
headerless_unified_diff, FileContext, FileId, FileMetadata, FileType, HeaderlessUnifiedDiff,
};
pub use crate::path::MononokePath;
pub use crate::repo::{BookmarkFreshness, RepoContext};
pub use crate::repo_write::create_changeset::{CreateChange, CreateCopyInfo};

View File

@ -5,6 +5,7 @@
* GNU General Public License version 2.
*/
mod test_file_diff;
mod test_history;
mod test_repo;
mod test_repo_bookmarks;

View File

@ -0,0 +1,74 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This software may be used and distributed according to the terms of the
* GNU General Public License version 2.
*/
use std::collections::BTreeMap;
use std::sync::Arc;
use anyhow::{Error, Result};
use fbinit::FacebookInit;
use tests_utils::drawdag::{changes, create_from_dag_with_changes};
use crate::{headerless_unified_diff, ChangesetId, CoreContext, Repo, RepoContext};
async fn init_repo(ctx: &CoreContext) -> Result<(RepoContext, BTreeMap<String, ChangesetId>)> {
let blob_repo = blobrepo_factory::new_memblob_empty(None)?;
let changesets = create_from_dag_with_changes(
ctx,
&blob_repo,
r##"
A-B-C
"##,
changes! {
"B" => |c| c.add_file("file", "test\nbefore\ndata\n").add_file("bin", "bin\0\x01"),
"C" => |c| c.add_file("file", "test\nafter\ndata\n").add_file("bin", "bin\0\x02"),
},
)
.await?;
let repo = Repo::new_test(ctx.clone(), blob_repo).await?;
let repo_ctx = RepoContext::new(ctx.clone(), Arc::new(repo)).await?;
Ok((repo_ctx, changesets))
}
#[fbinit::compat_test]
async fn file_diff(fb: FacebookInit) -> Result<(), Error> {
let ctx = CoreContext::test_mock(fb);
let (repo, changesets) = init_repo(&ctx).await?;
let b = repo
.changeset(changesets["B"])
.await?
.expect("changeset should exist");
let c = repo
.changeset(changesets["C"])
.await?
.expect("changeset should exist");
// Compare two regular files
let b_file = b.path("file")?.file().await?.expect("should be a file");
let c_file = c.path("file")?.file().await?.expect("should be a file");
let diff = headerless_unified_diff(&b_file, &c_file, 3).await?;
assert_eq!(
std::str::from_utf8(&diff.raw_diff)?,
concat!(
"@@ -1,3 +1,3 @@\n",
" test\n",
"-before\n",
"+after\n",
" data\n"
)
);
assert!(!diff.is_binary);
// Compare two binary files
let b_bin = b.path("bin")?.file().await?.expect("should be a file");
let c_bin = c.path("bin")?.file().await?.expect("should be a file");
let diff = headerless_unified_diff(&b_bin, &c_bin, 3).await?;
assert_eq!(std::str::from_utf8(&diff.raw_diff)?, "Binary files differ");
assert!(diff.is_binary);
Ok(())
}

View File

@ -123,6 +123,7 @@ impl_into_thrift_error!(service::TreeListExn);
impl_into_thrift_error!(service::FileExistsExn);
impl_into_thrift_error!(service::FileInfoExn);
impl_into_thrift_error!(service::FileContentChunkExn);
impl_into_thrift_error!(service::FileDiffExn);
impl_into_thrift_error!(service::CommitLookupXrepoExn);
impl_into_thrift_error!(service::RepoListHgManifestExn);

View File

@ -13,8 +13,9 @@ use futures::try_join;
use itertools::Itertools;
use maplit::btreemap;
use mononoke_api::{
ChangesetContext, ChangesetId, ChangesetPathContext, FileMetadata, FileType, MononokeError,
PushrebaseOutcome, RepoContext, TreeEntry, TreeId, TreeSummary, UnifiedDiff,
ChangesetContext, ChangesetId, ChangesetPathContext, FileMetadata, FileType,
HeaderlessUnifiedDiff, MononokeError, PushrebaseOutcome, RepoContext, TreeEntry, TreeId,
TreeSummary, UnifiedDiff,
};
use source_control as thrift;
use std::collections::{BTreeMap, BTreeSet};
@ -130,6 +131,15 @@ impl IntoResponse<thrift::Diff> for UnifiedDiff {
}
}
impl IntoResponse<thrift::Diff> for HeaderlessUnifiedDiff {
fn into_response(self) -> thrift::Diff {
thrift::Diff::raw_diff(thrift::RawDiff {
raw_diff: Some(self.raw_diff),
is_binary: self.is_binary,
})
}
}
#[async_trait]
impl AsyncIntoResponse<Option<thrift::FilePathInfo>> for ChangesetPathContext {
async fn into_response(self) -> Result<Option<thrift::FilePathInfo>, errors::ServiceError> {

View File

@ -6,10 +6,11 @@
*/
use context::CoreContext;
use mononoke_api::{headerless_unified_diff, FileId};
use source_control as thrift;
use crate::errors;
use crate::from_request::check_range_and_convert;
use crate::from_request::{check_range_and_convert, FromRequest};
use crate::into_response::IntoResponse;
use crate::source_control_impl::SourceControlServiceImpl;
use crate::specifiers::SpecifierExt;
@ -65,4 +66,29 @@ impl SourceControlServiceImpl {
(_repo, None) => Err(errors::file_not_found(file.description()).into()),
}
}
/// Compare a file with another file.
pub(crate) async fn file_diff(
&self,
ctx: CoreContext,
file: thrift::FileSpecifier,
params: thrift::FileDiffParams,
) -> Result<thrift::FileDiffResponse, errors::ServiceError> {
let context_lines = params.context as usize;
let (repo, base_file) = self.repo_file(ctx, &file).await?;
let base_file = base_file.ok_or_else(|| errors::file_not_found(file.description()))?;
let other_file_id = FileId::from_request(&params.other_file_id)?;
let other_file = repo
.file(other_file_id)
.await?
.ok_or_else(|| errors::file_not_found(other_file_id.to_string()))?;
let diff = headerless_unified_diff(&other_file, &base_file, context_lines)
.await?
.into_response();
Ok(thrift::FileDiffResponse { diff })
}
}

View File

@ -243,6 +243,17 @@ impl AddScubaParams for thrift::FileExistsParams {}
impl AddScubaParams for thrift::FileInfoParams {}
impl AddScubaParams for thrift::FileDiffParams {
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
scuba.add(
"other_file",
faster_hex::hex_string(&self.other_file_id).expect("hex_string should never fail"),
);
scuba.add("param_format", self.format.to_string());
scuba.add("param_context", self.context);
}
}
impl AddScubaParams for thrift::TreeListParams {
fn add_scuba_params(&self, scuba: &mut ScubaSampleBuilder) {
scuba.add("param_offset", self.offset);

View File

@ -499,6 +499,11 @@ impl SourceControlService for SourceControlServiceThriftImpl {
params: thrift::FileContentChunkParams,
) -> Result<thrift::FileChunk, service::FileContentChunkExn>;
async fn file_diff(
file: thrift::FileSpecifier,
params: thrift::FileDiffParams,
) -> Result<thrift::FileDiffResponse, service::FileDiffExn>;
async fn repo_create_commit(
repo: thrift::RepoSpecifier,
params: thrift::RepoCreateCommitParams,