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

Fix dependency issue for git hook tasks #837

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ntkoopman
Copy link

When running one of the Git hook tasks together with another task, Gradle will complain with an error:

Reason: Task ':addKtlintCheckGitPreCommitHook' uses this output of task ':samples:kotlin-reporter-using:compileKotlin' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed.

This is caused by @InputDirectory marking the entire project root and contents as input, so any task that modifies any file in the project will cause Gradle to add an implicit dependency.

You can see this behaviour by running ./gradlew compileKotlin addKtlintCheckGitPreCommitHook.

Build result on the first commit of this PR, to show it failing: https://github.com/ntkoopman/ktlint-gradle/actions/runs/13514290368/job/37760683637

This is the same issue as #545, except this PR keeps the inputs.

@get:InputDirectory
internal val rootDirectory: DirectoryProperty = objectFactory.directoryProperty().apply {
set(project.rootDir)
@get:Input
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the correct way to make an input depend on the path and not the contents 👍

}

@TaskAction
fun installHook() {
val repo = RepositoryBuilder().findGitDir(projectDir.get().asFile).setMustExist(false).build()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have some work to do in terms of properly making this task depend on the git directory as an input. But i think this PR is fine for now and we can refine later

@wakingrufus
Copy link
Collaborator

Thanks for the contribution! please update the changelog file (see the failed check for this for details). I am approving the ci checks to run, and once everything passes, I will open to merging this

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.

2 participants