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

Throw GradleException if .git directory is missing #162

Merged
merged 1 commit into from Mar 14, 2021

Conversation

joschi
Copy link
Contributor

@joschi joschi commented Mar 13, 2021

GitPropertiesPluginTests#testGenerateWithMissingGitRepoShouldFail incorrectly asserted that every exception would be a RepositoryNotFoundException while in reality the thrown exception was about Assert.fail() not being imported and no such method could be found.

While fixing the test, I learned that JGit wouldn't throw a RepositoryNotFoundException in every case but happily walk up the directory tree until it found a .git directory starting from the current working directory with findGitDir() (see BaseRepositoryBuilder#findGitDir(), BaseRepositoryBuilder#findGitDir(java.io.File)).

Instead of relying on this behavior, GenerateGitPropertiesTask#getGeneratedProperties() will now throw a GradleException if dotGitDirectory is null to get the expected behavior.

As an alternative, GitPropertiesPluginTests#testGenerateWithMissingGitRepoShouldFail could be removed if the default behavior of JGit (walking up the directory tree until a .git directory was found, starting from the current working directory) is the desired behavior.

`GitPropertiesPluginTests#testGenerateWithMissingGitRepoShouldFail`
incorrectly asserted that every exception would be a
`RepositoryNotFoundException` while in reality the exception was about
`Assert.fail()` not being imported and no such method could be found.

While fixing the test, I learned that JGit wouldn't throw a
`RepositoryNotFoundException` in every case but happily walk up the
directory tree until it found a `.git` directory with
`BaseRepositoryBuilder#findGitDir()` (see [1], [2]).

Instead of relying on this behavior,
`GenerateGitPropertiesTask#getGeneratedProperties()` will now throw a
`GradleException` if `dotGitDirectory` is `null` to get the expected
behavior.

[1]: https://archive.eclipse.org/jgit/site/3.7.0.201502260915-r/org.eclipse.jgit/apidocs/org/eclipse/jgit/lib/BaseRepositoryBuilder.html#findGitDir()
[2]: https://archive.eclipse.org/jgit/site/3.7.0.201502260915-r/org.eclipse.jgit/apidocs/org/eclipse/jgit/lib/BaseRepositoryBuilder.html#findGitDir(java.io.File)
@joschi
Copy link
Contributor Author

joschi commented Mar 13, 2021

The early access build of Java 17 (17-ea+13-1000) doesn't seem to like me. 😅

@tha2015
Copy link
Collaborator

tha2015 commented Mar 14, 2021

Thanks. It looks great to me.

@tha2015 tha2015 merged commit 6431367 into n0mer:master Mar 14, 2021
@joschi joschi deleted the exception-when-dotgit-missing branch March 14, 2021 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants