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.

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

Reset instead of merge to update branches #3219

Merged
merged 1 commit into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ final class FileGitAlg[F[_]](config: GitCfg)(implicit

override def hasConflicts(repo: File, branch: Branch, base: Branch): F[Boolean] = {
val tryMerge = git_("merge", "--no-commit", "--no-ff", branch.name)(repo)
val abortMerge = git_("merge", "--abort")(repo).void
val abortMerge = git_("merge", "--abort")(repo).attempt.void

returnToCurrentBranch(repo) {
checkoutBranch(repo, base) >> F.guarantee(tryMerge, abortMerge).attempt.map(_.isLeft)
Expand All @@ -114,41 +114,14 @@ final class FileGitAlg[F[_]](config: GitCfg)(implicit
git("rev-parse", "--verify", branch.name)(repo)
.flatMap(lines => F.fromEither(Sha1.from(lines.mkString("").trim)))

override def mergeTheirs(repo: File, branch: Branch): F[Option[Commit]] =
for {
before <- latestSha1(repo, Branch.head)
_ <- git_("merge", "--strategy-option=theirs", sign, branch.name)(repo).void
.handleErrorWith { throwable =>
// Resolve CONFLICT (modify/delete) by deleting unmerged files:
for {
unmergedFiles <- git("diff", "--name-only", "--diff-filter=U")(repo)
_ <- Nel
.fromList(unmergedFiles.filter(_.nonEmpty))
.fold(F.raiseError[Unit](throwable))(_.traverse_(file => git_("rm", file)(repo)))
_ <- git_("commit", "--all", "--no-edit", sign)(repo)
} yield ()
}
after <- latestSha1(repo, Branch.head)
} yield Option.when(before =!= after)(Commit(after))

override def push(repo: File, branch: Branch): F[Unit] =
git_("push", "--force", "--set-upstream", "origin", branch.name)(repo).void

override def removeClone(repo: File): F[Unit] =
fileAlg.deleteForce(repo)

override def revertChanges(repo: File, base: Branch): F[Option[Commit]] = {
val range = dotdot(base, Branch.head)
git("log", "--pretty=format:%h %p", range)(repo).flatMap { commitsWithParents =>
val commitsUntilMerge = commitsWithParents.map(_.split(' ')).takeWhile(_.length === 2)
val commits = commitsUntilMerge.flatMap(_.headOption)
if (commits.isEmpty) F.pure(None)
else {
val msg = CommitMsg(s"Revert commit(s) " + commits.mkString(", "))
git_("revert" :: "--no-commit" :: commits: _*)(repo) >> commitAllIfDirty(repo, msg)
}
}
}
override def resetHard(repo: File, base: Branch): F[Unit] =
git_("reset", "--hard", base.name)(repo).void

override def setAuthor(repo: File, author: Author): F[Unit] =
for {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,11 @@ trait GenGitAlg[F[_], Repo] {

def latestSha1(repo: Repo, branch: Branch): F[Sha1]

/** Merges `branch` into the current branch using `theirs` as merge strategy option. */
def mergeTheirs(repo: Repo, branch: Branch): F[Option[Commit]]

def push(repo: Repo, branch: Branch): F[Unit]

def removeClone(repo: Repo): F[Unit]

def revertChanges(repo: Repo, base: Branch): F[Option[Commit]]
def resetHard(repo: Repo, base: Branch): F[Unit]

def setAuthor(repo: Repo, author: Author): F[Unit]

Expand Down Expand Up @@ -152,17 +149,14 @@ trait GenGitAlg[F[_], Repo] {
override def latestSha1(repo: A, branch: Branch): F[Sha1] =
f(repo).flatMap(self.latestSha1(_, branch))

override def mergeTheirs(repo: A, branch: Branch): F[Option[Commit]] =
f(repo).flatMap(self.mergeTheirs(_, branch))

override def push(repo: A, branch: Branch): F[Unit] =
f(repo).flatMap(self.push(_, branch))

override def removeClone(repo: A): F[Unit] =
f(repo).flatMap(self.removeClone)

override def revertChanges(repo: A, base: Branch): F[Option[Commit]] =
f(repo).flatMap(self.revertChanges(_, base))
override def resetHard(repo: A, base: Branch): F[Unit] =
f(repo).flatMap(self.resetHard(_, base))

override def setAuthor(repo: A, author: Author): F[Unit] =
f(repo).flatMap(self.setAuthor(_, author))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@
gitAlg.returnToCurrentBranch(data.repo) {
gitAlg.checkoutBranch(data.repo, data.updateBranch) >>
shouldBeUpdated(data).ifM(
ifTrue = mergeAndApplyAgain(number, data),
ifTrue = resetAndApplyAgain(number, data),

Check warning on line 270 in modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala

View check run for this annotation

Codecov / codecov/patch

modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala#L270

Added line #L270 was not covered by tests
ifFalse = (Ignored: ProcessResult).pure
)
}
Expand All @@ -293,22 +293,19 @@
result.flatMap { case (update, msg) => logger.info(msg).as(update) }
}

private def mergeAndApplyAgain(number: PullRequestNumber, data: UpdateData): F[ProcessResult] =
private def resetAndApplyAgain(number: PullRequestNumber, data: UpdateData): F[ProcessResult] =
for {
_ <- logger.info(
s"Merge branch ${data.baseBranch.name} into ${data.updateBranch.name} and apply again"
s"Reset ${data.updateBranch.name} to ${data.baseBranch.name} and apply again"
)
maybeRevertCommit <- gitAlg.revertChanges(data.repo, data.baseBranch)
maybeMergeCommit <- gitAlg.mergeTheirs(data.repo, data.baseBranch)
_ <- gitAlg.resetHard(data.repo, data.baseBranch)

Check warning on line 301 in modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala

View check run for this annotation

Codecov / codecov/patch

modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala#L301

Added line #L301 was not covered by tests
edits <- data.update.on(
update = editAlg.applyUpdate(data.repoData, _),
grouped = _.updates.flatTraverse(editAlg.applyUpdate(data.repoData, _))
)
editCommits = edits.flatMap(_.commits)
commits = maybeRevertCommit.toList ++ maybeMergeCommit.toList ++ editCommits
result <- pushCommits(data, commits)
result <- pushCommits(data, editCommits)

Check warning on line 307 in modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala

View check run for this annotation

Codecov / codecov/patch

modules/core/src/main/scala/org/scalasteward/core/nurture/NurtureAlg.scala#L307

Added line #L307 was not covered by tests
requestData <- preparePullRequest(data, edits)
_ <- forgeApiAlg.updatePullRequest(number: PullRequestNumber, data.repo, requestData)
} yield result

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import cats.effect.IO
import cats.syntax.all._
import munit.CatsEffectSuite
import org.scalasteward.core.TestInstances.ioLogger
import org.scalasteward.core.git.FileGitAlgTest.{ioAuxGitAlg, ioGitAlg, master}
import org.scalasteward.core.git.FileGitAlgTest.{
conflictsNo,
conflictsYes,
ioAuxGitAlg,
ioGitAlg,
master
}
import org.scalasteward.core.io.FileAlgTest.ioFileAlg
import org.scalasteward.core.io.ProcessAlgTest.ioProcessAlg
import org.scalasteward.core.io.{FileAlg, ProcessAlg, WorkspaceAlg}
Expand Down Expand Up @@ -47,7 +53,7 @@ class FileGitAlgTest extends CatsEffectSuite {
for {
_ <- ioAuxGitAlg.createRepo(repo)
_ <- ioAuxGitAlg.createConflict(repo)
authors <- ioGitAlg.branchAuthors(repo, Branch("conflicts-no"), master)
authors <- ioGitAlg.branchAuthors(repo, conflictsNo, master)
_ = assertEquals(authors, List("'Bot Doe'"))
} yield ()
}
Expand Down Expand Up @@ -157,57 +163,37 @@ class FileGitAlgTest extends CatsEffectSuite {
for {
_ <- ioAuxGitAlg.createRepo(repo)
_ <- ioAuxGitAlg.createConflict(repo)
c1 <- ioGitAlg.hasConflicts(repo, Branch("conflicts-yes"), master)
c2 <- ioGitAlg.hasConflicts(repo, Branch("conflicts-no"), master)
c1 <- ioGitAlg.hasConflicts(repo, conflictsYes, master)
c2 <- ioGitAlg.hasConflicts(repo, conflictsNo, master)
_ = assertEquals((c1, c2), (true, false))
} yield ()
}

test("mergeTheirs") {
val repo = rootDir / "mergeTheirs"
test("isMerged") {
val repo = rootDir / "isMerged"
for {
_ <- ioAuxGitAlg.createRepo(repo)
_ <- ioAuxGitAlg.createConflict(repo)
branch = Branch("conflicts-yes")
c1 <- ioGitAlg.hasConflicts(repo, branch, master)
m1 <- ioGitAlg.isMerged(repo, master, branch)
_ <- ioGitAlg.checkoutBranch(repo, branch)
_ <- ioGitAlg.mergeTheirs(repo, master)
c2 <- ioGitAlg.hasConflicts(repo, branch, master)
m2 <- ioGitAlg.isMerged(repo, master, branch)
_ = assertEquals((c1, m1, c2, m2), (true, false, false, true))
} yield ()
}

test("mergeTheirs: CONFLICT (modify/delete)") {
val repo = rootDir / "mergeTheirs-modify-delete"
for {
_ <- ioAuxGitAlg.createRepo(repo)
_ <- ioAuxGitAlg.createConflictFileRemovedOnMaster(repo)
branch = Branch("conflicts-yes")
c1 <- ioGitAlg.hasConflicts(repo, branch, master)
m1 <- ioGitAlg.isMerged(repo, master, branch)
_ <- ioGitAlg.checkoutBranch(repo, branch)
_ <- ioGitAlg.mergeTheirs(repo, master)
c2 <- ioGitAlg.hasConflicts(repo, branch, master)
m2 <- ioGitAlg.isMerged(repo, master, branch)
_ = assertEquals((c1, m1, c2, m2), (true, false, false, true))
m1 <- ioGitAlg.isMerged(repo, conflictsNo, master)
_ <- ioAuxGitAlg.git("merge", conflictsNo.name)(repo)
m2 <- ioGitAlg.isMerged(repo, conflictsNo, master)
_ = assertEquals((m1, m2), (false, true))
} yield ()
}

test("revertChanges") {
val repo = rootDir / "revertChanges"
test("resetHard") {
val repo = rootDir / "resetHard"
for {
_ <- ioAuxGitAlg.createRepo(repo)
_ <- ioAuxGitAlg.createConflict(repo)
branch = Branch("conflicts-yes")
branch = conflictsYes
c1 <- ioGitAlg.hasConflicts(repo, branch, master)
d1 <- ioGitAlg.branchesDiffer(repo, master, branch)
_ <- ioGitAlg.checkoutBranch(repo, branch)
_ <- ioGitAlg.revertChanges(repo, master)
_ <- ioGitAlg.resetHard(repo, master)
c2 <- ioGitAlg.hasConflicts(repo, branch, master)
d2 <- ioGitAlg.branchesDiffer(repo, master, branch)
_ = assertEquals((c1, d1, c2, d2), (true, true, false, true))
_ = assertEquals((c1, d1, c2, d2), (true, true, false, false))
} yield ()
}

Expand All @@ -217,7 +203,9 @@ class FileGitAlgTest extends CatsEffectSuite {
}

object FileGitAlgTest {
val master: Branch = Branch("master")
private val master: Branch = Branch("master")
private val conflictsNo: Branch = Branch("conflicts-no")
private val conflictsYes: Branch = Branch("conflicts-yes")

final class AuxGitAlg[F[_]](implicit
fileAlg: FileAlg[F],
Expand Down Expand Up @@ -250,13 +238,13 @@ object FileGitAlgTest {
_ <- fileAlg.writeFile(repo / "file2", "file2, line1")
_ <- addFiles(repo, repo / "file1", repo / "file2")
// work on conflicts-no
_ <- gitAlg.createBranch(repo, Branch("conflicts-no"))
_ <- gitAlg.createBranch(repo, conflictsNo)
_ <- fileAlg.writeFile(repo / "file3", "file3, line1")
_ <- git("add", "file3")(repo)
_ <- gitAlg.commitAll(repo, CommitMsg("Add file3 on conflicts-no"))
_ <- gitAlg.checkoutBranch(repo, master)
// work on conflicts-yes
_ <- gitAlg.createBranch(repo, Branch("conflicts-yes"))
_ <- gitAlg.createBranch(repo, conflictsYes)
_ <- fileAlg.writeFile(repo / "file2", "file2, line1\nfile2, line2 on conflicts-yes")
_ <- git("add", "file2")(repo)
_ <- gitAlg.commitAll(repo, CommitMsg("Modify file2 on conflicts-yes"))
Expand All @@ -274,7 +262,7 @@ object FileGitAlgTest {
_ <- fileAlg.writeFile(repo / "file2", "file2, line1")
_ <- addFiles(repo, repo / "file1", repo / "file2")
// work on conflicts-yes
_ <- gitAlg.createBranch(repo, Branch("conflicts-yes"))
_ <- gitAlg.createBranch(repo, conflictsYes)
_ <- fileAlg.writeFile(repo / "file2", "file2, line1\nfile2, line2 on conflicts-yes")
_ <- git("add", "file2")(repo)
_ <- gitAlg.commitAll(repo, CommitMsg("Modify file2 on conflicts-yes"))
Expand Down