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

Pre-push hook shows an error #205

Closed
ghost opened this issue May 13, 2021 · 10 comments · Fixed by #218
Closed

Pre-push hook shows an error #205

ghost opened this issue May 13, 2021 · 10 comments · Fixed by #218
Labels

Comments

@ghost
Copy link

ghost commented May 13, 2021

In Windows 10, you can see hook install task generate hook looks like this.

#!/bin/sh
set -e
##### KOTLINTER HOOK START #####
##### KOTLINTER 3.4.4 #####
GRADLEW=C:\Users\rlcks\workspace\freelec\gradlew.bat
if ! $GRADLEW lintKotlin ; then
    echo 1>&2 "\nlintKotlin found problems, running formatKotlin; commit the result and re-push"
    $GRADLEW formatKotlin
    exit 1
fi
##### KOTLINTER HOOK END #####

And this shows error with message below.

.git/hooks/pre-push: line 6: C:Usersrlcksworkspacefreelecgradlew.bat: command not found
\nlintKotlin found problems, running formatKotlin; commit the result and re-push
.git/hooks/pre-push: line 8: C:Usersrlcksworkspacefreelecgradlew.bat: command not found
error: failed to push some refs to 'https://github.com/nkgcp/freelec.git'

It seems like \ in the hook file is recognized as escape character. Changing \ to \\ or / makes it works fine.

@jeremymailen
Copy link
Owner

Thank you for the error report @nkgcp! Will get it fixed soon.

@rvplauborg
Copy link

Whats the status on this - we also ran into it and it would be nice to avoid it for newcomers to the project :-)

@jeremymailen
Copy link
Owner

Sorry it's slipped through the cracks.
The fastest way would be for someone to submit a PR -- especially someone who works in a windows environment and can quickly verify the fix.
Otherwise I will try to get to it.

@rvplauborg
Copy link

rvplauborg commented Aug 21, 2021

Nothing to be sorry about. I am not sure how to verify a fix. I think a possible solution could be altering gradlew.path to gradlew.invariantSeparatorsPath in the gradleCommand val of GitHookTask.kt. But not sure how to verify it.

@jeremymailen
Copy link
Owner

jeremymailen commented Aug 21, 2021

Nothing to be sorry about. I am not sure how to verify a fix. I think a possible solution could be altering gradlew.path to gradlew.invariantSeparatorsPath in the gradleCommand val of GitHookTask.kt. But not sure how to verify it.

On your pull request branch you can bump version, publish to maven local, and then in your test project put mavenLocal() first in the repo list and pull in the version of kotlinter built by your PR. At least that's what I usually do.

The solution might be straightforward, but I always like to see a shell script run for real on the OS, make sure it doesn't need quoting or anything else like that -- still returns an exit code.

@rvplauborg
Copy link

rvplauborg commented Aug 23, 2021

Hmm.. Cant seem to make it work, gradle cannot find the plugin.

plugins {
   ...
   id("org.jmailen.kotlinter") version "3.5.0-test" // my test version
}
repositories {
    mavenLocal()
    mavenCentral()
}

But I can find it in my ~.m2/repository folder.

@ghost
Copy link
Author

ghost commented Aug 23, 2021

Hmm.. Cant seem to make it work, gradle cannot find the plugin.

plugins {
   ...
   id("org.jmailen.kotlinter") version "3.5.0-test" // my test version
}
repositories {
    mavenLocal()
    mavenCentral()
}

But I can find it in my ~.m2/repository folder.

May you try putting this in settings.gradle.kts file?

pluginManagement { 
    repositories { 
        mavenLocal()
        gradlePluginPortal()
    }
}

@jeremymailen
Copy link
Owner

Or the instructions for multi-module in the readme will work:

buildscript {
    repositories {
        mavenLocal()
        ...
    }
    dependencies {
        classpath("org.jmailen.gradle:kotlinter-gradle:3.5.0-test")
    }
}

apply(plugin = "org.jmailen.kotlinter")

@bespaltovyj
Copy link
Contributor

@rvplauborg thanks for your idea for solution this problem.
I have issued a pull request along with it. Sorry, if you wanted to do it yourself.

#218

@rvplauborg
Copy link

Awesome that it worked. Didn’t get around to doing it, so good you stepped in. @bespaltovyj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants