Move git utilities into a dedicated crate#15564
Conversation
|
@codex review this |
f9a12b9 to
56b3e16
Compare
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
56b3e16 to
12948a4
Compare
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
1 similar comment
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05fdd24124
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
codex-rs/Cargo.lock
Outdated
| [[package]] | ||
| name = "tar" | ||
| version = "0.4.45" | ||
| version = "0.4.44" |
There was a problem hiding this comment.
Upgrade tar to a non-vulnerable version
The lockfile downgrades tar to 0.4.44, and this same change adds cargo-deny ignores for RUSTSEC-2026-0067/0068 (codex-rs/deny.toml:79-80). That reintroduces known archive parsing/extraction vulnerabilities while suppressing automated detection. Keep tar on a patched release (>=0.4.45) and avoid suppressing these advisories in this refactor.
Useful? React with 👍 / 👎.
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
|
@codex review this |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review this |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Create codex-git-utils, move the existing git helpers into it, and migrate the GitInfo helpers out of core so rollout can depend on the shared crate without keeping its own git info module. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Keep the git-utils extraction behavior-neutral by restoring the protocol-owned GitInfo type and reverting rename-only string drift.\n\nCo-authored-by: Codex <noreply@openai.com>
Revert the unrelated registry refresh from the git-utils extraction and keep only the minimal workspace lockfile changes needed for the moved crate.\n\nCo-authored-by: Codex <noreply@openai.com>
Remove the git-utils dev-dependency cycle by letting codex-core own the git-info tests again while the shared crate keeps the runtime implementation.\n\nCo-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
c49a339 to
a18d733
Compare
|
|
||
| #[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, TS)] | ||
| pub struct GitInfo { | ||
| /// Current commit hash (SHA) |
There was a problem hiding this comment.
Comment clarify that this is the hex string representation.
Why don't we continue to use the newtype:
pub struct GitSha(pub String);
|
|
||
| impl GitSha { | ||
| pub fn new(sha: &str) -> Self { | ||
| Self(sha.to_string()) |
There was a problem hiding this comment.
It feels a little irresponsible to me that we don't verify sha is a hex string of 40 digits, but maybe there is a reason why we didn't...
codex-rs/git-utils/src/info.rs
Outdated
| pub struct GitInfo { | ||
| /// Current commit hash (SHA) | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub commit_hash: Option<String>, |
There was a problem hiding this comment.
Can/should this be GitSha?
codex-rs/git-utils/src/info.rs
Outdated
| } | ||
|
|
||
| let git_dir_path = canonicalize_or_raw(resolve_path(&repo_root, &PathBuf::from(git_dir_rel))); | ||
| let git_dir_path = canonicalize_or_raw({ |
There was a problem hiding this comment.
Feels like a job for AbsolutePathBuf?
codex-git-utilsand move the shared git helpers into it with file moves preserved for diff readabilityGitInfohelpers out ofcoreso stacked rollout work can depend on the shared crate without carrying its own git info module