Skip to content

Commit

Permalink
Fix issue #34 - reporting protected refs with slashes in the name
Browse files Browse the repository at this point in the history
Eg this crashed:

$ java -jar bfg-1.11.1.jar --strip-blobs-bigger-than 100K -p 'feature/worker-prototype' my-repo.git

The code wasn't expecting branch names with a slash (because, like,
obviously those are ug-lee) - and it's not possible to create filenames
that contain slashes:

#34
  • Loading branch information
rtyley committed Mar 21, 2014
1 parent 3053e1d commit 4ec5be6
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ class CLIReporter(repo: Repository) extends Reporter {
abbreviate(diffEntries.view.map(diffDetails).map(fileInfo), "...").foreach {
dirtyFile => println("\t- " + dirtyFile)
}
val diffFile = protectedDirtDir / s"${report.revObject.shortName}-${protectorRevs.mkString("_")}.csv"

val protectorRefsFileNameSafe = protectorRevs.mkString("_").replace(protectedDirtDir.separator, "-")
val diffFile = protectedDirtDir / s"${report.revObject.shortName}-${protectorRefsFileNameSafe}.csv"

diffFile.writeStrings(diffEntries.map {
diffEntry =>
Expand Down
Binary file not shown.
10 changes: 9 additions & 1 deletion bfg/src/test/scala/com/madgag/git/bfg/cli/MainSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class MainSpec extends Specification {
"not change commits unnecessarily" in new unpackedRepo("/sample-repos/exampleWithInitialCleanHistory.git.zip") {
implicit val r = reader

ensureInvariant(commitHist take 2) {
ensureInvariant(commitHist() take 2) {
ensureRemovalOf(commitHistory(haveCommitWhereObjectIds(contain(be_==(abbrId("294f")))).atLeastOnce)) {
run("--strip-blobs-bigger-than 1K")
}
Expand All @@ -58,6 +58,14 @@ class MainSpec extends Specification {
}
}

"not crash when facing a protected branch containing a slash in it's name" in new unpackedRepo("/sample-repos/branchNameWithASlash.git.zip") {
ensureInvariant(haveRef("feature/slashes-are-ugly", haveFile("bar"))) {
ensureRemovalOf(commitHistoryFor("master")(haveFile("bar").atLeastOnce)) {
run("--delete-files bar --protect-blobs-from feature/slashes-are-ugly")
}
}
}

"strip blobs by id" in new unpackedRepo("/sample-repos/example.git.zip") {
implicit val r = reader

Expand Down
38 changes: 29 additions & 9 deletions bfg/src/test/scala/com/madgag/git/bfg/test/unpackedRepo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package com.madgag.git.bfg.test

import scala.collection.convert.wrapAsScala._
import org.eclipse.jgit.lib.{Constants, ObjectReader, ObjectId, Repository}
import org.eclipse.jgit.revwalk.RevCommit
import org.specs2.matcher.{MustThrownMatchers, Matcher}
import org.eclipse.jgit.revwalk.{RevObject, RevTree, RevCommit}
import org.specs2.matcher.{Matcher, MustThrownMatchers}
import com.madgag.git._
import com.madgag.git.test._
import org.specs2.specification.Scope
Expand Down Expand Up @@ -32,18 +32,28 @@ class unpackedRepo(filePath: String) extends Scope with MustThrownMatchers {
}.toSet
}

def haveFile(name: String) = be_==(name).atLeastOnce ^^ { (c: RevCommit) => treeEntryNames(c, !_.isSubtree) }
def haveFile(name: String) = haveTreeEntry(name, !_.isSubtree)

def haveFolder(name: String) = be_==(name).atLeastOnce ^^ { (c: RevCommit) => treeEntryNames(c, _.isSubtree) }
def haveFolder(name: String) = haveTreeEntry(name, _.isSubtree)

def treeEntryNames(c: RevCommit, p: TreeWalk => Boolean): Seq[String] =
c.getTree.walk(postOrderTraversal = true).withFilter(p).map(_.getNameString).toList
def haveTreeEntry(name: String, p: TreeWalk => Boolean) = be_==(name).atLeastOnce ^^ { (treeish: ObjectId) =>
treeOrBlobPointedToBy(treeish.asRevObject) match {
case Right(tree) => treeEntryNames(tree, p)
case Left(blob) => Seq.empty[String]
}
}

def treeEntryNames(t: RevTree, p: TreeWalk => Boolean): Seq[String] =
t.walk(postOrderTraversal = true).withFilter(p).map(_.getNameString).toList

def run(options: String) {
Main.main(options.split(' ') :+ repo.getDirectory.getAbsolutePath)
}

def commitHist(implicit repo: Repository) = repo.git.log.all.call.toSeq.reverse
def commitHist(specificRefs: String*)(implicit repo: Repository) = {
val logCommand = repo.git.log
if (specificRefs.isEmpty) logCommand.all else specificRefs.foldLeft(logCommand)((lc, ref) => lc.add(repo.resolve(ref)))
}.call.toSeq.reverse

def haveCommitWhereObjectIds(boom: Matcher[Traversable[ObjectId]])(implicit reader: ObjectReader): Matcher[RevCommit] = boom ^^ {
(c: RevCommit) => c.getTree.walk().map(_.getObjectId(0)).toSeq
Expand All @@ -53,8 +63,12 @@ class unpackedRepo(filePath: String) extends Scope with MustThrownMatchers {
(r: Repository) => r resolve (refName) aka s"Ref [$refName]"
}

def commitHistory(histMatcher: Matcher[Seq[RevCommit]]): Matcher[Repository] = histMatcher ^^ {
(r: Repository) => commitHist(r)
def commitHistory(histMatcher: Matcher[Seq[RevCommit]]) = histMatcher ^^ {
r: Repository => commitHist()(r)
}

def commitHistoryFor(refs: String*)(histMatcher: Matcher[Seq[RevCommit]]) = histMatcher ^^ {
r: Repository => commitHist(refs:_*)(r)
}

def ensureRemovalOfBadEggs[S,T](expr : => Traversable[S], exprResultMatcher: Matcher[Traversable[S]])(block: => T) = {
Expand All @@ -80,4 +94,10 @@ class unpackedRepo(filePath: String) extends Scope with MustThrownMatchers {
block
f mustEqual originalValue
}

def ensureInvariant[T](repoMatcher: Matcher[Repository])(block: => T) = {
repo must repoMatcher
block
repo must repoMatcher
}
}

0 comments on commit 4ec5be6

Please sign in to comment.