-
Notifications
You must be signed in to change notification settings - Fork 444
feat: unstable SHA256 support #1206
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
base: main
Are you sure you want to change the base?
Changes from all commits
e452ab9
414b840
f8bf444
17d0f51
81cbee9
8c17e75
aa866f6
79d5e2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -33,10 +33,11 @@ jobs: | |||||
| - name: Install Rust (rustup) | ||||||
| run: rustup update ${{ matrix.rust }} --no-self-update && rustup default ${{ matrix.rust }} | ||||||
| shell: bash | ||||||
| - run: cargo test --locked | ||||||
| - run: cargo test --features https,ssh | ||||||
| - run: cargo run -p systest | ||||||
| - run: cargo run -p systest --features unstable-sha256 | ||||||
| - run: cargo test --locked | ||||||
| - run: cargo test --features https,ssh | ||||||
| - run: cargo test --features unstable-sha256 | ||||||
|
Contributor
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. Can we also make sure this covers networking (and examples)? Maybe something like:
Suggested change
|
||||||
| - run: cargo test -p git2-curl | ||||||
|
|
||||||
| rustfmt: | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |||||
| #![deny(warnings)] | ||||||
|
|
||||||
| use clap::Parser; | ||||||
| use git2::ObjectFormat; | ||||||
| use git2::{Error, Repository, RepositoryInitMode, RepositoryInitOptions}; | ||||||
| use std::path::{Path, PathBuf}; | ||||||
|
|
||||||
|
|
@@ -40,6 +41,9 @@ struct Args { | |||||
| #[structopt(name = "perms", long = "shared")] | ||||||
| /// permissions to create the repository with | ||||||
| flag_shared: Option<String>, | ||||||
| #[structopt(name = "object-format", long, value_parser = parse_object_format)] | ||||||
| /// object format to use (sha1 or sha256, requires unstable-sha256 feature) | ||||||
|
Contributor
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.
Suggested change
|
||||||
| flag_object_format: Option<ObjectFormat>, | ||||||
| } | ||||||
|
|
||||||
| fn run(args: &Args) -> Result<(), Error> { | ||||||
|
|
@@ -48,6 +52,7 @@ fn run(args: &Args) -> Result<(), Error> { | |||||
| && args.flag_template.is_none() | ||||||
| && args.flag_shared.is_none() | ||||||
| && args.flag_separate_git_dir.is_none() | ||||||
| && args.flag_object_format.is_none() | ||||||
| { | ||||||
| Repository::init(&path)? | ||||||
| } else { | ||||||
|
|
@@ -68,6 +73,12 @@ fn run(args: &Args) -> Result<(), Error> { | |||||
| if let Some(ref s) = args.flag_shared { | ||||||
| opts.mode(parse_shared(s)?); | ||||||
| } | ||||||
|
|
||||||
| #[cfg(feature = "unstable-sha256")] | ||||||
| if let Some(format) = args.flag_object_format { | ||||||
| opts.object_format(format); | ||||||
| } | ||||||
|
|
||||||
| Repository::init_opts(&path, &opts)? | ||||||
| }; | ||||||
|
|
||||||
|
|
@@ -136,6 +147,15 @@ fn parse_shared(shared: &str) -> Result<RepositoryInitMode, Error> { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| fn parse_object_format(format: &str) -> Result<ObjectFormat, Error> { | ||||||
| match format { | ||||||
| "sha1" => Ok(ObjectFormat::Sha1), | ||||||
| #[cfg(feature = "unstable-sha256")] | ||||||
| "sha256" => Ok(ObjectFormat::Sha256), | ||||||
| _ => Err(Error::from_str("object format must be 'sha1' or 'sha256'")), | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| fn main() { | ||||||
| let args = Args::parse(); | ||||||
| match run(&args) { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -310,16 +310,38 @@ impl Diff<'static> { | |
| /// two trees, however there may be subtle differences. For example, | ||
| /// a patch file likely contains abbreviated object IDs, so the | ||
| /// object IDs parsed by this function will also be abbreviated. | ||
| /// | ||
| /// This parses the diff assuming SHA1 object IDs. Use | ||
| /// [`Diff::from_buffer_ext`] to specify a different format. | ||
| pub fn from_buffer(buffer: &[u8]) -> Result<Diff<'static>, Error> { | ||
| Self::from_buffer_ext(buffer, crate::ObjectFormat::Sha1) | ||
|
Contributor
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. I can't make suggestions on the unmodified line, but I suggest adding an import for |
||
| } | ||
|
|
||
| /// Reads the contents of a git patch file into a `git_diff` object, | ||
| /// with a specific object format. | ||
| /// | ||
| /// See [`Diff::from_buffer`] for more details. | ||
| pub fn from_buffer_ext( | ||
| buffer: &[u8], | ||
| format: crate::ObjectFormat, | ||
| ) -> Result<Diff<'static>, Error> { | ||
| crate::init(); | ||
| let mut diff: *mut raw::git_diff = std::ptr::null_mut(); | ||
| let data = buffer.as_ptr() as *const c_char; | ||
| let len = buffer.len(); | ||
| unsafe { | ||
| // NOTE: Doesn't depend on repo, so lifetime can be 'static | ||
|
Contributor
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. this explanation of why things can be 'static is lost |
||
| try_call!(raw::git_diff_from_buffer( | ||
| &mut diff, | ||
| buffer.as_ptr() as *const c_char, | ||
| buffer.len() | ||
| )); | ||
| #[cfg(not(feature = "unstable-sha256"))] | ||
| { | ||
| let _ = format; | ||
| try_call!(raw::git_diff_from_buffer(&mut diff, data, len)); | ||
| } | ||
| #[cfg(feature = "unstable-sha256")] | ||
| { | ||
| let mut opts: raw::git_diff_parse_options = std::mem::zeroed(); | ||
| opts.version = raw::GIT_DIFF_PARSE_OPTIONS_VERSION; | ||
| opts.oid_type = format.raw(); | ||
| try_call!(raw::git_diff_from_buffer(&mut diff, data, len, &mut opts)); | ||
| } | ||
| Ok(Diff::from_raw(diff)) | ||
| } | ||
| } | ||
|
|
@@ -1552,6 +1574,8 @@ impl DiffPatchidOptions { | |
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| #[cfg(feature = "unstable-sha256")] | ||
| use crate::Diff; | ||
| use crate::{DiffLineType, DiffOptions, Oid, Signature, Time}; | ||
| use std::borrow::Borrow; | ||
| use std::fs::File; | ||
|
|
@@ -1568,7 +1592,7 @@ mod tests { | |
| assert_eq!(stats.deletions(), 0); | ||
| assert_eq!(stats.files_changed(), 0); | ||
| let patchid = diff.patchid(None).unwrap(); | ||
| assert_ne!(patchid, Oid::zero()); | ||
| assert_ne!(patchid, Oid::ZERO_SHA1); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -1858,4 +1882,37 @@ mod tests { | |
|
|
||
| assert_eq!(result.unwrap_err().code(), crate::ErrorCode::User); | ||
| } | ||
|
|
||
| #[test] | ||
| #[cfg(feature = "unstable-sha256")] | ||
| fn diff_sha256() { | ||
| let (_td, repo) = crate::test::repo_init_sha256(); | ||
| let diff = repo.diff_tree_to_workdir(None, None).unwrap(); | ||
| assert_eq!(diff.deltas().len(), 0); | ||
| let stats = diff.stats().unwrap(); | ||
| assert_eq!(stats.insertions(), 0); | ||
| assert_eq!(stats.deletions(), 0); | ||
| assert_eq!(stats.files_changed(), 0); | ||
| let patchid = diff.patchid(None).unwrap(); | ||
|
|
||
| // Verify SHA256 OID (32 bytes) | ||
| assert_eq!(patchid.as_bytes().len(), 32); | ||
| } | ||
|
|
||
| #[test] | ||
| #[cfg(feature = "unstable-sha256")] | ||
| fn diff_from_buffer_sha256() { | ||
| // Minimal patch with SHA256 OID (64 chars) | ||
| let patch = b"diff --git a/file.txt b/file.txt | ||
| index 0000000000000000000000000000000000000000000000000000000000000000..1111111111111111111111111111111111111111111111111111111111111111 100644 | ||
| --- a/file.txt | ||
| +++ b/file.txt | ||
| @@ -1 +1 @@ | ||
| -old | ||
| +new | ||
| "; | ||
|
|
||
| let diff = Diff::from_buffer_ext(patch, crate::ObjectFormat::Sha256).unwrap(); | ||
| assert_eq!(diff.deltas().len(), 1); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
Moving these breaks the
--lockedcheck since the earliercargo runwill update the lockfile. Can you either move them back, or add--lockedto the other commands?View changes since the review