checkstyle
Issue #818: do shallow clone while retaining checkout abilitiy to specific SHA
Resolves #818
This PR reuses the earlier PR on shallow clone PR, while reverted retaining checkout abilitiy to specific SHA.
I tested this PR on main repo https://github.com/checkstyle/checkstyle/pull/14662, by following the footprints of what already has been done. I am attaching the test results below:
Successful build while retaining checkout to specific SHA: https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25238/workflows/51a420a4-ff1f-4137-89c0-802b6ecfcd81/jobs/577549
Testing Checkstyle started
[delete] Deleting directory /home/circleci/project/.ci-temp/contribution/checkstyle-tester/src/main/java
Cloning git repository 'pmd' to repositories/pmd folder ...
Cloning into 'repositories/pmd'...
Cloning git repository 'pmd' - completed
Running command: git fetch --tags
Resetting git sources to commit 'pmd_releases/6.21.0'
Running command: git reset --hard refs/tags/pmd_releases/6.21.0
HEAD is now at a28e9e22e5 [maven-release-plugin] prepare release pmd_releases/6.21.0
pmd is synchronized
Sucessful build testing Shallow Clone: https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25238/workflows/51a420a4-ff1f-4137-89c0-802b6ecfcd81/jobs/577567
Testing Checkstyle started
[delete] Deleting directory /home/circleci/project/.ci-temp/contribution/checkstyle-tester/src/main/java
Shallow clone git repository 'apache-struts' to repositories/apache-struts folder ...
Cloning into 'repositories/apache-struts'...
Cloning git repository 'apache-struts' - completed
I realised in the event of shallow clone, this PR still fails to checkout to specific SHA. I am trying to resolve it.
UPDATE: I did cross-check, with a test PR on main repo, and the logs seem to have restored the ability to point to a specific SHA.
@relentless-pursuit , please reply done to review items, if you are ready for review
@relentless-pursuit , please reply done to review items, if you are ready for review
Yes, @romani. I am yet to do the testing of this PR as I have modified the code. Once, I attach the test results, I will respond to the review suggestions/comments and proceed further.
Please share all evidence of testing in PR description.
Items:
I have updated the description section of the PR. please let me know your feedback and what else needs to be done here. Also, the PR has some useless print statements. I will remove them once I have some clarity on where the PR is headed.
@relentless-pursuit please link to your testing PR in the main repo in the PR description
@relentless-pursuit please link to your testing PR in the main repo in the PR description
done
Last from me, please rerun test and share CI output/link:
@nrmancuso below is the link to the successful build by running my test PR on main repo.
https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25222/workflows/83042d6f-0fde-45e4-ba37-ee8f80fc8cc3/jobs/576859
Last from me, please rerun test and share CI output/link:
@nrmancuso below is the link to the successful build by running my test PR on main repo.
https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25222/workflows/83042d6f-0fde-45e4-ba37-ee8f80fc8cc3/jobs/576859
Please keep testing evidence in the PR description to have one source of truth.
Build is successful, but it doesn't look like we are shallow cloning: https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25222/workflows/83042d6f-0fde-45e4-ba37-ee8f80fc8cc3/jobs/576859?invite=true#step-103-33656_59
Cloning git repository 'pmd' to repositories/pmd folder ...
I suggest running this groovy script locally with a small project (maybe your own, something with 1 file or whatever) to debug to get a faster feedback loop.
Last from me, please rerun test and share CI output/link:
@nrmancuso below is the link to the successful build by running my test PR on main repo. https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25222/workflows/83042d6f-0fde-45e4-ba37-ee8f80fc8cc3/jobs/576859
Please keep testing evidence in the PR description to have one source of truth.
Build is successful, but it doesn't look like we are shallow cloning: https://app.circleci.com/pipelines/github/checkstyle/checkstyle/25222/workflows/83042d6f-0fde-45e4-ba37-ee8f80fc8cc3/jobs/576859?invite=true#step-103-33656_59
Cloning git repository 'pmd' to repositories/pmd folder ...I suggest running this groovy script locally with a small project (maybe your own, something with 1 file or whatever) to debug to get a faster feedback loop.
@nrmancuso I have updated the testing evidence in the PR description. Also, i assume you meant testing shallow clone in local. If i understood you right, I have attahced a log file showcasing occurence of shallow clone and other details in the PR description. Please let me know if it is in sync of what you were expecting. Thank you.
UPDATE: For convenience, used a gist showcing the logs rather than attaching a log txt file.
Why aren’t we shallow cloning in the test PR/CI execution that you linked?
Why aren’t we shallow cloning in the test PR/CI execution that you linked?
@nrmancuso there was a mistake on my part. I wasn't referring to the right build link when testing this PR on main repo. Infact, the earlier code was failing when tryting to fetch a specific tag with an error message as below.
error: cannot update ref 'refs/heads/pmd_releases/6.21.0': trying to write non-commit object 4c719b23b5980f12f2c73d946ff72b2a8d8fc472 to branch 'refs/heads/pmd_releases/6.21.0'
Therefore, I had to a new section to treat tags appropriately.
def isTag(commitId, gitRepo) {
def tagCheckCmd = "git tag -l $commitId".execute(null, gitRepo)
tagCheckCmd.waitFor()
return tagCheckCmd.text.trim().equals(commitId)
}
Now, as per my understanding, I am being able to replicate shallow cloning and restoring the checkout ability. I have added the proof in the PR description. Please have a look. Sorry for the confusion.
I am seeing no documentation update or CI update in this repo to use the command and ensure it continues to function properly in the future. https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#command-line-arguments
I believe we reverted this commit previously and it had other changes in it.
@relentless-pursuit , please add to your PR doc update https://github.com/checkstyle/contribution/commit/f10f82a9c036fc4565b4868d7cc8b0a3fe6eea2d#diff-1d82d916db8f81ee95ff589f11bde9990f0b83f950957a48e4a573d77e6fff5fR75
I am seeing no documentation update or CI update in this repo to use the command and ensure it continues to function properly in the future. https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#command-line-arguments
I believe we reverted this commit previously and it had other changes in it.
done
I am seeing no ... CI update in this repo to use the command and ensure it continues to function properly in the future
This is not done. I see the other PR in the main repo being pinged to me, but that isn't what I was originally asking.
Are we going to use this function in this repo were we test out everything else for PRs here? Is the PR in the main repo going to be merged once this is merged?
I am seeing no ... CI update in this repo to use the command and ensure it continues to function properly in the future
This is not done. I see the other PR in the main repo being pinged to me, but that isn't what I was originally asking.
Are we going to use this function in this repo were we test out everything else for PRs here? Is the PR in the main repo going to be merged once this is merged?
The other PR is a testing PR, which has been configured to test this specific PR. That is not going to be merged. Pardon me if am wrong, but what i understood is that the script in main repo i.e. validation.sh uses the diff.groovy in contribution repo. Hence, i followed the earlier suit of testing.
Then we should update the CI here to use this option to ensure we are testing and somewhat validating future updates.
Then we should update the CI here to use this option to ensure we are testing and somewhat validating future updates.
Aren't we doing that in diff.groovy which is a part of the CI by adding a new option i.e. shallowClone? Let me know if am missing something.
Show me then where we are using it in this repo's CI. Your configuration for this new parameter says it is not required and defaults to false, meaning it won't use a shallow clone unless it is specifically written out.
https://github.com/checkstyle/checkstyle/pull/14662/files#diff-a74bb028b327d2ef179514069ca703e16a92ca5d0a18670c2d3daa356e032d7cR911
Please add to https://github.com/checkstyle/contribution/blob/3decd5c38c3e225b9fd00f299199843b25aeb8b0/appveyor.yml#L85
Please add to
https://github.com/checkstyle/contribution/blob/3decd5c38c3e225b9fd00f299199843b25aeb8b0/appveyor.yml#L85
done.
@romani, the https://ci.appveyor.com/project/Checkstyle/contribution/builds/49778146 is failing after adding the parameter of shallowClone.
failure was:
Exception calling "DownloadFile" with "2" argument(s): "The remote server returned an error: (404) Not Found."
At line:3 char:3
+ (new-object System.Net.WebClient).DownloadFile(
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : NotSpecified: (:) [], MethodInvocationException
+ FullyQualifiedErrorId : WebException
Exception calling "ExtractToDirectory" with "2" argument(s): "Could not find file 'C:\maven-bin.zip'."
At line:7 char:3
+ [System.IO.Compression.ZipFile]::ExtractToDirectory("C:\maven-bin.z ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : NotSpecified: (:) [], MethodInvocationException
+ FullyQualifiedErrorId : FileNotFoundException
Command executed with exception: Exception calling "ExtractToDirectory" with "2" argument(s): "Could not find file 'C:\maven-bin.zip'."
unrelated. restated