Skip to content

Commit

Permalink
git-checkout: Support branchless commits
Browse files Browse the repository at this point in the history
When creating a database from a git repository, bender currently fetches
all branches and tags from the remote before checking out the specified
reference. However, it is possible that a commit does not belong to a
branch or a tag. For instance, GitHub creates PR merge commits and
stores their references in `refs/pull`. To checkout such a branchless
commit, it must first be fetched explicitly.

Therefore, when creating the database, explicitly fetch the specified
reference in addition to all branches and tags (those are still needed
e.g. for `bender update`).

An open corner case is when `Bender.lock` is updated with a branchless
commit hash after creating the database, but the manifest is not. This
will skip fetching altogether and the database might not contain the
desired branchless commit. A possible solution would be to track the
modification time of `Bender.lock`; see `manifest_mtime`. However, since
this is not the intended use of bender and, in a similar form, already
possible today, I chose not to address this for now.

Context: The pulp-action
[integrate](https://github.com/pulp-platform/pulp-actions/tree/main/integrate)
currently does not work for `pull_request` events, since the run's
context (the PR merge commit) is not accessible by the dependent's CI
due to the issue addressed by this PR. (see
https://github.com/pulp-platform/cva6/actions/runs/10512893871/job/29127223222)

Signed-off-by: Nils Wistoff <[email protected]>
  • Loading branch information
niwis authored and micprog committed Oct 12, 2024
1 parent a6b1c8b commit d408ac1
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
7 changes: 7 additions & 0 deletions src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ impl<'git, 'ctx> Git<'ctx> {
.map(|_| ())
}

/// Fetch the specified ref of a remote.
pub async fn fetch_ref(self, remote: &str, reference: &str) -> Result<()> {
self.spawn_with(|c| c.arg("fetch").arg(remote).arg(reference))
.await
.map(|_| ())
}

/// Stage all local changes.
pub async fn add_all(self) -> Result<()> {
self.spawn_with(|c| c.arg("add").arg("--all"))
Expand Down
21 changes: 18 additions & 3 deletions src/sess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> {
}
DependencySource::Path(_) => Ok(DependencyVersions::Path),
DependencySource::Git(ref url) => {
let db = self.git_database(&dep.name, url, force_fetch).await?;
let db = self.git_database(&dep.name, url, force_fetch, None).await?;
self.git_versions_func(db)
.await
.map(DependencyVersions::Git)
Expand All @@ -477,6 +477,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> {
name: &str,
url: &str,
force_fetch: bool,
fetch_ref: Option<&str>,
) -> Result<Git<'ctx>> {
// TODO: Make the assembled future shared and keep it in a lookup table.
// Then use that table to return the future if it already exists.
Expand Down Expand Up @@ -533,6 +534,13 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> {
.and_then(|_| git.spawn_with(|c| c.arg("init").arg("--bare")))
.and_then(|_| git.spawn_with(|c| c.arg("remote").arg("add").arg("origin").arg(url)))
.and_then(|_| git.fetch("origin"))
.and_then(|_| async {
if let Some(reference) = fetch_ref {
git.fetch_ref("origin", reference).await
} else {
Ok(())
}
})
.await
.map_err(move |cause| {
if url3.contains("git@") {
Expand All @@ -559,6 +567,13 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> {
Ok(())
})
.and_then(|_| git.fetch("origin"))
.and_then(|_| async {
if let Some(reference) = fetch_ref {
git.fetch_ref("origin", reference).await
} else {
Ok(())
}
})
.await
.map_err(move |cause| {
if url3.contains("git@") {
Expand Down Expand Up @@ -841,7 +856,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> {
// branches or tags for shallow clones.
let tag_name_0 = format!("bender-tmp-{}", revision);
let tag_name_1 = tag_name_0.clone();
let git = self.git_database(name, url, false).await?;
let git = self.git_database(name, url, false, Some(revision)).await?;
git.spawn_with(move |c| c.arg("tag").arg(tag_name_0).arg(revision).arg("--force"))
.map_err(move |cause| {
warnln!("Please ensure the commits are available on the remote or run bender update");
Expand Down Expand Up @@ -1041,7 +1056,7 @@ impl<'io, 'sess: 'io, 'ctx: 'sess> SessionIo<'sess, 'ctx> {
(DepSrc::Git(url), DepVer::Git(rev)) => {
let dep_name = self.sess.intern_string(dep.name.as_str());
// TODO MICHAERO: May need proper chaining using and_then
let db = self.git_database(&dep.name, url, false).await?;
let db = self.git_database(&dep.name, url, false, None).await?;
let entries = db.list_files(rev, Some("Bender.yml")).await?;
let data = match entries.into_iter().next() {
None => Ok(None),
Expand Down

0 comments on commit d408ac1

Please sign in to comment.