Skip to content

Commit

Permalink
Issue checkstyle#818: do shallow clone while retaining checkout abili…
Browse files Browse the repository at this point in the history
…tiy to specific SHA
  • Loading branch information
piyush kumar sadangi authored and romani committed May 9, 2024
1 parent 4a12d92 commit d6509ae
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 17 deletions.
10 changes: 5 additions & 5 deletions .ci/validation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ checkstyle-tester-diff-groovy-patch)
sed -i'' 's/^guava/#guava/' ../.ci-temp/projects-to-test-on.properties
sed -i'' 's/#checkstyle/checkstyle/' ../.ci-temp/projects-to-test-on.properties
groovy diff.groovy -l ../.ci-temp/projects-to-test-on.properties \
-c my_check.xml -b master -p patch-branch -r ../.ci-temp/checkstyle
-c my_check.xml -b master -p patch-branch -r ../.ci-temp/checkstyle -h
;;

checkstyle-tester-diff-groovy-base-patch)
Expand All @@ -50,7 +50,7 @@ checkstyle-tester-diff-groovy-base-patch)
git checkout -b patch-branch
cd ../../checkstyle-tester
groovy diff.groovy -l projects-to-test-on.properties \
-bc my_check.xml -pc my_check.xml -b master -p patch-branch -r ../.ci-temp/checkstyle
-bc my_check.xml -pc my_check.xml -b master -p patch-branch -r ../.ci-temp/checkstyle -h
;;

checkstyle-tester-diff-groovy-patch-only)
Expand All @@ -59,7 +59,7 @@ checkstyle-tester-diff-groovy-patch-only)
git checkout -b patch-branch
cd ../../checkstyle-tester
groovy diff.groovy -l projects-to-test-on.properties \
-pc my_check.xml -p patch-branch -r ../.ci-temp/checkstyle -m single
-pc my_check.xml -p patch-branch -r ../.ci-temp/checkstyle -m single --useShallowClone
;;

checkstyle-tester-diff-groovy-regression-single)
Expand All @@ -76,7 +76,7 @@ checkstyle-tester-diff-groovy-regression-single)
groovy ./diff.groovy --listOfProjects projects-to-test-on.properties \
-pc ../../../checkstyle-tester/diff-groovy-regression-config.xml \
-r ../../checkstyle -xm "-Dcheckstyle.failsOnError=false" \
-m single -p master
-m single -p master -h

# Run report with current branch
cd ../../../checkstyle-tester/
Expand All @@ -85,7 +85,7 @@ checkstyle-tester-diff-groovy-regression-single)
rm -rf reports repositories
groovy ./diff.groovy --listOfProjects projects-to-test-on.properties \
-pc diff-groovy-regression-config.xml -r ../.ci-temp/checkstyle/ \
-m single -p master -xm "-Dcheckstyle.failsOnError=false"
-m single -p master -xm "-Dcheckstyle.failsOnError=false" -h

cd ..
# We need to ignore file paths below, since they will be different between reports
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ environment:
CMD3: " && cd checkstyle && git checkout -b patch-branch"
CMD4: " "
CMD5: " && cd ..\\checkstyle-tester "
CMD6: " && groovy diff.groovy -l projects-to-test-on.properties -bc my_check.xml -pc my_check.xml -b master -p patch-branch -r C:\\projects\\contribution\\checkstyle -s"
CMD6: " && groovy diff.groovy -l projects-to-test-on.properties -bc my_check.xml -pc my_check.xml -b master -p patch-branch -r C:\\projects\\contribution\\checkstyle -s -h"

build_script:
- ps: >
Expand Down
10 changes: 10 additions & 0 deletions checkstyle-tester/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ would add `--extraMvnRegressionOptions "-Dmaven.site.skip=true"` to `diff.groovy
(optional, default is false). This option is
used for files that are not compilable or that Checkstyle cannot parse.

**useShallowClone** (h) - this option tells `diff.groovy` to use shallow clone for repositories
defined in the file
specified for the `--listOfProjects (-l)` argument. This option is a toggle (does not require an argument)
and convenient for cases when internet is not stable and it is better to clone minimal required sources.
Shallow cloning cannot be used for projects that reference SHA commit and, by default,
fallback to using normal cloning. ATTENTION: if you change tag/branch in config for project
that previously cloned shallowly there will be error to checkout to new tag/branch
(Example: `fatal: Could not parse object 'c2ac5b90a467aedb04b52ae50a99e83207d847b3'.`), to resovle
problem you need to remove impacted project folder(s) from repositories directory.

## Outputs

When the script finishes its work the following directory structure will be created
Expand Down
85 changes: 74 additions & 11 deletions checkstyle-tester/diff.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def getCliOptions(args) {
+ 'config file (required if baseConfig and patchConfig are not secified)')
g(longOpt: 'allowExcludes', required: false, 'Whether to allow excludes specified in the list of ' \
+ 'projects (optional, default is false)')
h(longOpt: 'useShallowClone', 'Enable shallow cloning')
l(longOpt: 'listOfProjects', args: 1, required: true, argName: 'path',
'Path to file which contains projects to test on (required)')
s(longOpt: 'shortFilePaths', required: false, 'Whether to save report file paths' \
Expand Down Expand Up @@ -264,6 +265,7 @@ def generateCheckstyleReport(cfg) {
def checkstyleConfig = cfg.checkstyleCfg
def checkstyleVersion = cfg.checkstyleVersion
def allowExcludes = cfg.allowExcludes
def useShallowClone = cfg.useShallowClone
def listOfProjectsFile = new File(cfg.listOfProjects)
def projects = listOfProjectsFile.readLines()
def extraMvnRegressionOptions = cfg.extraMvnRegressionOptions
Expand Down Expand Up @@ -292,7 +294,11 @@ def generateCheckstyleReport(cfg) {
if (repoType == 'local') {
copyDir(repoUrl, getOsSpecificPath("$srcDir", "$repoName"))
} else {
cloneRepository(repoName, repoType, repoUrl, commitId, reposDir)
if (useShallowClone && !isGitSha(commitId)) {
shallowCloneRepository(repoName, repoType, repoUrl, commitId, reposDir)
} else {
cloneRepository(repoName, repoType, repoUrl, commitId, reposDir)
}
copyDir(getOsSpecificPath("$reposDir", "$repoName"), getOsSpecificPath("$srcDir", "$repoName"))
}
runMavenExecution(srcDir, excludes, checkstyleConfig,
Expand Down Expand Up @@ -340,16 +346,15 @@ def getCommitSha(commitId, repoType, srcDestinationDir) {
return sha.replace('\n', '')
}

def getCloneCmd(repoType, repoUrl, srcDestinationDir) {
def cloneCmd = ''
switch (repoType) {
case 'git':
cloneCmd = "git clone $repoUrl $srcDestinationDir"
break
default:
throw new IllegalArgumentException("Error! Unknown $repoType repository.")
def shallowCloneRepository(repoName, repoType, repoUrl, commitId, srcDir) {
def srcDestinationDir = getOsSpecificPath("$srcDir", "$repoName")
if (!Files.exists(Paths.get(srcDestinationDir))) {
def cloneCmd = getCloneShallowCmd(repoType, repoUrl, srcDestinationDir, commitId)
println "Shallow clone $repoType repository '$repoName' to $srcDestinationDir folder ..."
executeCmdWithRetry(cloneCmd)
println "Cloning $repoType repository '$repoName' - completed\n"
}
return cloneCmd
println "$repoName is synchronized"
}

def cloneRepository(repoName, repoType, repoUrl, commitId, srcDir) {
Expand All @@ -365,6 +370,10 @@ def cloneRepository(repoName, repoType, repoUrl, commitId, srcDir) {
def lastCommitSha = getLastProjectCommitSha(repoType, srcDestinationDir)
def commitIdSha = getCommitSha(commitId, repoType, srcDestinationDir)
if (lastCommitSha != commitIdSha) {
if (!isGitSha(commitId)) {
// If commitId is a branch or tag, fetch more data and then reset
fetchAdditionalData(repoType, srcDestinationDir, commitId)
}
def resetCmd = getResetCmd(repoType, commitId)
println "Resetting $repoType sources to commit '$commitId'"
executeCmd(resetCmd, new File("$srcDestinationDir"))
Expand All @@ -373,6 +382,51 @@ def cloneRepository(repoName, repoType, repoUrl, commitId, srcDir) {
println "$repoName is synchronized"
}

def getCloneCmd(repoType, repoUrl, srcDestinationDir) {
if ('git'.equals(repoType)) {
return "git clone $repoUrl $srcDestinationDir"
}
throw new IllegalArgumentException("Error! Unknown $repoType repository.")
}

def getCloneShallowCmd(repoType, repoUrl, srcDestinationDir, commitId) {
if ('git'.equals(repoType)) {
return "git clone --depth 1 --branch $commitId $repoUrl $srcDestinationDir"
}
throw new IllegalArgumentException("Error! Unknown repository type: $repoType")
}

def fetchAdditionalData(repoType, srcDestinationDir, commitId) {
String fetchCmd = ''
if ('git'.equals(repoType)) {
if (isGitSha(commitId)) {
fetchCmd = "git fetch"
} else {
// Check if commitId is a tag and handle accordingly
if (isTag(commitId, new File(srcDestinationDir))) {
fetchCmd = "git fetch --tags"
} else {
fetchCmd = "git fetch origin $commitId:$commitId"
}
}
} else {
throw new IllegalArgumentException("Error! Unknown $repoType repository.")
}

executeCmd(fetchCmd, new File(srcDestinationDir))
}

def isTag(commitId, gitRepo) {
def tagCheckCmd = "git tag -l $commitId".execute(null, gitRepo)
tagCheckCmd.waitFor()
return tagCheckCmd.text.trim().equals(commitId)
}

// it is not very accurate match, but in case of mismatch we will do full clone
def isGitSha(value) {
return value ==~ /[0-9a-f]{5,40}/
}

def executeCmdWithRetry(cmd, dir = new File("").getAbsoluteFile(), retry = 5) {
def osSpecificCmd = getOsSpecificCmd(cmd)
def left = retry
Expand Down Expand Up @@ -708,7 +762,11 @@ def getResetCmd(repoType, commitId) {
def resetCmd = ''
switch (repoType) {
case 'git':
resetCmd = "git reset --hard $commitId"
if (isGitSha(commitId)) {
resetCmd = "git reset --hard $commitId"
} else {
resetCmd = "git reset --hard refs/tags/$commitId"
}
break
default:
throw new IllegalArgumentException("Error! Unknown $repoType repository.")
Expand Down Expand Up @@ -755,6 +813,7 @@ class Config {
def checkstyleVersion
def sevntuVersion
def allowExcludes
def useShallowClone

Config(cliOptions) {
if (cliOptions.localGitRepo) {
Expand All @@ -767,6 +826,7 @@ class Config {

checkstyleVersion = cliOptions.checkstyleVersion
allowExcludes = cliOptions.allowExcludes
useShallowClone = cliOptions.useShallowClone

mode = cliOptions.mode
if (!mode) {
Expand Down Expand Up @@ -815,6 +875,7 @@ class Config {
destDir: tmpMasterReportsDir,
extraMvnRegressionOptions: extraMvnRegressionOptions,
allowExcludes:allowExcludes,
useShallowClone: useShallowClone,
]
}

Expand All @@ -827,6 +888,7 @@ class Config {
destDir: tmpPatchReportsDir,
extraMvnRegressionOptions: extraMvnRegressionOptions,
allowExcludes: allowExcludes,
useShallowClone: useShallowClone,
]
}

Expand All @@ -840,6 +902,7 @@ class Config {
shortFilePaths: shortFilePaths,
mode: mode,
allowExcludes: allowExcludes,
useShallowClone: useShallowClone,
]
}
}
Expand Down

0 comments on commit d6509ae

Please sign in to comment.