hipcheck
hipcheck copied to clipboard
Remove git2 dependency
Overview
This PR is split into 5 different commits to make it easier to review and revert, if there is a problem in the future. The commits are as follows:
- replace
git2-basedfetchimplementation withgix-based implementation - replace
git2-basedcloneimplementation withgix-based implementation - replace
git2-basedcheckoutimplementation withgix-based implementation - remove
git2dependency and remaininggit2code (logging shim, rustls transport shim...) - integrate
gixfunctionality with user facing output, so progress is visible to user
Benefits
- removed
git2andopenssldependencies - standardized on
gixforgitoperations, as it is already in use withmitre/gitplugin - considerable build time reduction (30% less time for debug build, 20% faster release builds)
- enhanced git UI output to be more informative
- faster
hc checkwhen agit cloneis needed (~5% fastermitre/hipcheck, ~8% fasterserde-rs/serde)
Comparison of User Facing Output
main git clone
remove-git2-dependency git clone
Build Timings
Note: All benchmarks were performed on an Ubuntu 24.04 machine with 8 CPUs and 32 GB of RAM
cargo build --workspace
Benchmark Command
cd /tmp && \
hyperfine --runs 2 \
--prepare 'cd ~/workspace/hipcheck/main && cargo clean' \
'cd ~/workspace/hipcheck/main && RUSTC_WRAPPER="" cargo build --workspace' \
--prepare 'cd ~/workspace/hipcheck/patrickjcasey/remove-git2-dependency && cargo clean' \
'cd ~/workspace/hipcheck/patrickjcasey/remove-git2-dependency && RUSTC_WRAPPER="" cargo build --workspace'
Results
cargo build --workspace --release
Benchmark Command
cd /tmp && \
hyperfine --runs 1 \
--prepare 'cd ~/workspace/hipcheck/main && cargo clean' \
'cd ~/workspace/hipcheck/main && RUSTC_WRAPPER="" cargo build --workspace --release' \
--prepare 'cd ~/workspace/hipcheck/patrickjcasey/remove-git2-dependency && cargo clean' \
'cd ~/workspace/hipcheck/patrickjcasey/remove-git2-dependency && RUSTC_WRAPPER="" cargo build --workspace --release'
Results
Cold Repo Analysis Benchmark
mitre/hipcheck cold start
testing a repo with ~700 commits
benchmark command
hyperfine --runs 3 \
--prepare 'rm -rf ~/.cache/hipcheck/clones/github/mitre && cd ~/workspace/hipcheck/main' \
'./target/release/hc check --policy ./config/local.release.Hipcheck.kdl https://github.com/mitre/hipcheck' \
--prepare 'rm -rf ~/.cache/hipcheck/clones/github/mitre && cd ~/workspace/hipcheck/patrickjcasey/remove-git2-dependency ' \
'./target/release/hc check --policy ./config/local.release.Hipcheck.kdl https://github.com/mitre/hipcheck'
benchmark result
serde-rs/serde cold start
testing a repo with ~4000 commits
benchmark command
hyperfine --runs 3 \
--prepare 'rm -rf ~/.cache/hipcheck/clones/github/serde-rs/serde&& cd ~/workspace/hipcheck/main' \
'./target/release/hc check --policy ./config/local.release.Hipcheck.kdl https://github.com/serde-rs/serde' \
--prepare 'rm -rf ~/.cache/hipcheck/clones/github/serde-rs/serde && cd ~/workspace/hipcheck/patrickjcasey/remove-git2-dependency ' \
'./target/release/hc check --policy ./config/local.release.Hipcheck.kdl https://github.com/serde-rs/serde'
benchmark result
numpy/numpy cold start
testing a repo with ~38000 commits
benchmark command
hyperfine --runs 3 \
--prepare 'rm -rf ~/.cache/hipcheck/clones/github/numpy/numpy && cd ~/workspace/hipcheck/main' \
'./target/release/hc check --policy ./config/local.release.Hipcheck.kdl https://github.com/numpy/numpy' \
--prepare 'rm -rf ~/.cache/hipcheck/clones/github/numpy/numpy && cd ~/workspace/hipcheck/patrickjcasey/remove-git2-dependency ' \
'./target/release/hc check --policy ./config/local.release.Hipcheck.kdl https://github.com/numpy/numpy'
benchmark result
Manual testing performed
Since this change impacts the way repos are retrieved and/or updated, I performed the following manual tests to verify the new implementation is working correctly:
hc run requiring a clone
rm -rf ~/.cache/hipcheck/clones/github/rustls/rustls
./target/debug/hc check --policy ./config/local.Hipcheck.kdl https://github.com/rustls/rustls
# manually checked commit in ~/.cache/hipcheck/clones/github/rustls/rustls matches commit hash of tip of main
hc run with existing repo, fetch & pull needed
cd ~/.cache/hipcheck/clones/github/rustls/rustls
git reset --hard HEAD~10
./target/debug/hc check --policy ./config/local.Hipcheck.kdl https://github.com/rustls/rustls
# manually checked commit in ~/.cache/hipcheck/clones/github/rustls/rustls matches commit hash of tip of main
Test PyPI Package without version
rm -rf ~/.cache/hipcheck/clones/github/tqdm
./target/debug/hc check --policy ./config/local.Hipcheck.kdl -t pypi tqdm
# manually checked commit in ~/.cache/hipcheck/clones/github/tqdm/tqdm matches commit hash of latest version tag on github (4.67.1)
Test PyPI package with version
rm -rf ~/.cache/hipcheck/clones/github/tqdm
./target/debug/hc check --policy ./config/local.Hipcheck.kdl -t pypi [email protected]
# manually checked commit in ~/.cache/hipcheck/clones/github/tqdm/tqdm matches commit hash of 4.50.0 tag on github
Test NPM Package without version
rm -rf ~/.cache/hipcheck/clones/github/tqdm
./target/debug/hc check --policy ./config/local.Hipcheck.kdl -t npm express
# manually checked commit in ~/.cache/hipcheck/clones/github/expressjs/express matches commit hash of latest version tag on github (5.0.1)
Test NPM Package without version
rm -rf ~/.cache/hipcheck/clones/github/tqdm
./target/debug/hc check --policy ./config/local.Hipcheck.kdl -t npm [email protected]
# manually checked commit in ~/.cache/hipcheck/clones/github/expressjs/express matches commit hash of 4.21.0 tag on github
Updated title and removed WIP from it since it's already marked as a draft and thus can't be accidentally merged, even by myself or Julian.
Great work on this Patrick. I'm getting:
failed to clone remote repository
An IO error occurred when talking to the server
do you remember if there were any steps you needed to take to get gix to work with MITRE certs?
I'm wondering if we don't have to do something like hipcheck/util/http/agent.rs
Talking with @alilleybrinker , we feel that the issue is Gix is not picking up native cert store, and is using webpki instead. We need to figure out how to do that without re-introducing openssl
Waiting for a response from the gitoxide project on the proper way of configuring the use of the system's certificates
Update: discussed during team sync today. Will continue waiting on guidance from the gitoxide folks rather than trying to maintain a patched fork with the support for certificate changes we need.
Patrick and I were able to make forward progress on this. Basically, we configured gix to use the reqwest backend, which uses rust-tls. reqwest has a feature rustls-tls-native-roots to use the native root cert store, which does the exact same thing we do in util/http/agent.rs. In order to cause reqwest to compile with this feature that gix does not set, we added reqwest as a non-optional dependency to Hipcheck core. When cargo builds everything, cargo's feature unification will cause the reqwest package that gix depends on to contain this feature.
The latest code is working on my MITRE Mac, meaning hipcheck is correctly using the system certificates
Closing given bandwidth limitations. Would love to revive in the future.