Skip to content
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.

Sign up for GitHub

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

Fix resetting a Git working tree to the latest revision of a branch #8170

Merged
merged 3 commits into from
Jan 25, 2024
Merged

Fix resetting a Git working tree to the latest revision of a branch #8170

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c1fd32
docs(Git): Improve some code comments
sschuberth Nov 2, 2023
34859a6
style(Git): Adjust formatting to ease setting line breakpoints
sschuberth Nov 2, 2023
4a0e912
fix(git): Do not rely on `FETCH_HEAD` to list the current branch first
sschuberth Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 40 additions & 15 deletions plugins/version-control-systems/git/src/main/kotlin/Git.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ import org.apache.logging.log4j.kotlin.logger

import org.eclipse.jgit.api.Git
import org.eclipse.jgit.api.LsRemoteCommand
import org.eclipse.jgit.api.ResetCommand
import org.eclipse.jgit.api.errors.GitAPIException
import org.eclipse.jgit.errors.UnsupportedCredentialItem
import org.eclipse.jgit.lib.Constants
import org.eclipse.jgit.lib.ObjectIdRef
import org.eclipse.jgit.lib.SymbolicRef
import org.eclipse.jgit.transport.CredentialItem
import org.eclipse.jgit.transport.CredentialsProvider
Expand All @@ -51,6 +53,7 @@ import org.ossreviewtoolkit.utils.common.CommandLineTool
import org.ossreviewtoolkit.utils.common.Os
import org.ossreviewtoolkit.utils.common.collectMessages
import org.ossreviewtoolkit.utils.common.safeMkdirs
import org.ossreviewtoolkit.utils.common.withoutPrefix
import org.ossreviewtoolkit.utils.ort.requestPasswordAuthentication
import org.ossreviewtoolkit.utils.ort.showStackTrace

Expand Down Expand Up @@ -194,18 +197,17 @@ class Git : VersionControlSystem(), CommandLineTool {
val fetch = git.fetch().setDepth(GIT_HISTORY_DEPTH)

// See https://git-scm.com/docs/gitrevisions#_specifying_revisions for how Git resolves ambiguous
// names.
runCatching { fetch.setRefSpecs(revision).call() }
.recoverCatching {
// Note that in contrast to branches / heads, Git does not namespace tags per remote.
val tagRefSpec = "+${Constants.R_TAGS}$revision:${Constants.R_TAGS}$revision"
fetch.setRefSpecs(tagRefSpec).call()
}
.recoverCatching {
val branchRefSpec = "+${Constants.R_HEADS}$revision:${Constants.R_REMOTES}origin/$revision"
fetch.setRefSpecs(branchRefSpec).call()
}
.getOrThrow()
// names. In particular, tag names have higher precedence than branch names.
runCatching {
fetch.setRefSpecs(revision).call()
}.recoverCatching {
// Note that in contrast to branches / heads, Git does not namespace tags per remote.
val tagRefSpec = "+${Constants.R_TAGS}$revision:${Constants.R_TAGS}$revision"
fetch.setRefSpecs(tagRefSpec).call()
}.recoverCatching {
val branchRefSpec = "+${Constants.R_HEADS}$revision:${Constants.R_REMOTES}origin/$revision"
fetch.setRefSpecs(branchRefSpec).call()
}.getOrThrow()
}.recoverCatching {
it.showStackTrace()

Expand All @@ -231,18 +233,41 @@ class Git : VersionControlSystem(), CommandLineTool {
} else {
workingTree.runGit("fetch", "--tags", "origin")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit-msg: Would it make sense to link 1 (I assume this commit introduced the bug) ?

Copy link
Member Author

@sschuberth sschuberth Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this commit introduced the bug

I'm actually not sure how guilty that commit really is. So far, running git reset --hard FETCH_HEAD in this scenario has always worked for me, and also @nnobelis was only able to reproduce this with a single of the hundreds repos they're scanning. So I assume this rather to be a Git server implementation bug, but it could be a JGit bug as well... I simply could not find any specs on the ordering of advertised refs.

So that's why I didn't explicitly refer to that commit, as I'm still unsure where the bug lies.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks for explaining!


// Do a dummy fetch call to create a JGit FetchResult based on the Git CLI call.
git.fetch().call()
}.onFailure {
it.showStackTrace()

logger.warn { "Failed to fetch everything: ${it.collectMessages()}" }
}.mapCatching {
// TODO: Migrate this to JGit once https://bugs.eclipse.org/bugs/show_bug.cgi?id=383772 is implemented.
}.mapCatching { fetchResult ->
// TODO: Migrate this to JGit once sparse checkout (https://bugs.eclipse.org/bugs/show_bug.cgi?id=383772) is
// implemented.
run("checkout", revision, workingDir = workingTree.workingDir)

// In case of a non-fixed revision (branch or tag) reset the working tree to ensure that the previously
// fetched changes are applied.
if (!isFixedRevision(workingTree, revision).getOrThrow()) {
run("reset", "--hard", "FETCH_HEAD", workingDir = workingTree.workingDir)
val refs = fetchResult.advertisedRefs.filterIsInstance<ObjectIdRef>()

val tags = refs.mapNotNull { ref ->
ref.name.withoutPrefix(Constants.R_TAGS)?.let { it to ref.objectId.name }
}.toMap()

logger.debug { "Tags fetched: ${tags.keys.joinToString()}" }

val branches = refs.mapNotNull { ref ->
ref.name.withoutPrefix(Constants.R_HEADS)?.let { it to ref.objectId.name }
}.toMap()

logger.debug { "Branches fetched: ${branches.keys.joinToString()}" }

// Tag names have higher precedence than branch names in case of ambiguity.
val resolvedRevision = checkNotNull(tags[revision] ?: branches[revision]) {
"Requested revision '$revision' not found in refs advertised by the server."
}

git.reset().setMode(ResetCommand.ResetType.HARD).setRef(resolvedRevision).call()
}

revision
Expand Down
Loading