diffplug / spotless

Keep your code spotless
Apache License 2.0
4.57k stars 458 forks source link

Better method for managing python black package #674

Open nedtwigg opened 4 years ago

nedtwigg commented 4 years ago

We are able to call black by shelling out to it on the system path, and we are able to cache its results by enforcing a version check on the binary. However, if it isn't on the path, or the version is wrong, we just show a helpful error message and let the user figure it out from there:

https://github.com/diffplug/spotless/blob/608e128381c89260f8a38fba985415c1f962ec7b/lib/src/main/java/com/diffplug/spotless/python/BlackStep.java#L65-L66

It would be nice if we could handle this better, or at least have better instructions. There are a few python plugins in the gradle ecosystem, I'll reach out to them to see if they are interested.

japoorv commented 2 years ago

When this is run on linux the black --version gives output

black, 22.6.0 (compiled: yes) Python (CPython) 3.9.6

This doesn't parse with the regex version (\S*) . We should ideally have a way to enforce this regex as well, since different executable version can have different way of showing versions too.

I see this has already been fixed, sorry for the confusion

leicmi commented 2 years ago

One way to "bundle" black would be to bootstrap a python-environment with black using gradle-python-envs from JetBrains: build.gradle:

plugins{
    id "com.jetbrains.python.envs" version "0.0.31"
}
envs {
    bootstrapDirectory = new File(buildDir, 'bootstrap')
    envsDirectory = file(buildDir)
    condaenv "py_env", "3.10", [condaPackage("black")]
}
task show_black_version(type: Exec, dependsOn: 'build_envs'){
    commandLine("./build/py_env/bin/black", "--version")
}

However, this always bootstraps MiniConda and the environment, independent of whether another python installation with black already exists. What do you think about such an approach?

nedtwigg commented 2 years ago

This would work for the Gradle plugin, but not the Maven one, and that's fine with me! One way to incorporate something like this would be to add documentation to the manual on how to glue the two plugins together: https://github.com/diffplug/spotless/tree/main/plugin-gradle#black

and then do black().pathToExe('./build/py_env/bin/black')

leicmi commented 2 years ago

Unfortunately, it seems like this approach won't work. I tried to set a dependency on the python-environment using spotlessPythonApply.dependsOn('build_envs') (in build.gradle), however as of now ForeignExe.confirmVersionAndGetAbsolutePath() (of spotless) seems to be executed before any Gradle tasks.

nedtwigg commented 2 years ago

Ahh, yes. confirmVersionAndGetAbsolutePath is called as part of up-to-date checking... I really hope the Gradle depenency model learns to play nice with PyPI/NPM/RubyGems/etc someday...

nedtwigg commented 1 year ago

@ibabel-chwy if you have a specific problem you should open a new issue and include the requested info (version, logs, etc)

devinrsmith commented 3 months ago

Is there a way to say "use whatever version of black you find on the path"? I would have assumed that's what happened by using the default:

spotless {
    python {
        target '**/*.py'
        black()
    }
}

but it appears the default that spotless sets (22.3.0) is a necessity?

https://github.com/diffplug/spotless/blob/gradle/6.25.0/lib/src/main/java/com/diffplug/spotless/python/BlackStep.java#L38-L40

> Task :py-server:spotlessPython FAILED                                                                                                                                                                              
Step 'black' found problem in 'deephaven/experimental/__init__.py':                                                                                                                                                  
You specified version 22.3.0, but Spotless found 24.2.0                                                                                                                                                              
Try running {@code pip install --force-reinstall black==22.3.0}, or else specify {@code black('24.2.0')} to Spotless                                                                                                 
  github issue to handle this better: https://github.com/diffplug/spotless/issues/674                                                                                                                                
> arguments: [/home/devin/.local/bin/black, --version]                                                                                                                                                               
> exit code: 0                                                                                                                                                                                                       
>    stdout: (below)                                                                                                                                                                                                 
> black, 24.2.0 (compiled: yes)                                                                                                                                                                                      
> Python (CPython) 3.12.4                                                                                                                                                                                            

java.lang.RuntimeException: You specified version 22.3.0, but Spotless found 24.2.0                                                                                                                                  
Try running {@code pip install --force-reinstall black==22.3.0}, or else specify {@code black('24.2.0')} to Spotless                                                                                                 
  github issue to handle this better: https://github.com/diffplug/spotless/issues/674                                                                                                                                
> arguments: [/home/devin/.local/bin/black, --version]                                                                                                                                                               
> exit code: 0                                                                                                                                                                                                       
>    stdout: (below)                                                                                                                                                                                                 
> black, 24.2.0 (compiled: yes)                                                                                                                                                                                      
> Python (CPython) 3.12.4                                                                                                                                                                                            

        at com.diffplug.spotless.ForeignExe.exceptionFmt(ForeignExe.java:133)                                                                                                                                        
        at com.diffplug.spotless.ForeignExe.wrongVersion(ForeignExe.java:121)                                                                                                                                        
        at com.diffplug.spotless.ForeignExe.confirmVersionAndGetAbsolutePath(ForeignExe.java:110)                                                                                                                    
        at com.diffplug.spotless.python.BlackStep$State.format(BlackStep.java:88)                                                                                                                                    
        at com.diffplug.spotless.FormatterFunc$Closeable$2.apply(FormatterFunc.java:97)                                                                                                                              
        at com.diffplug.spotless.FormatterStepImpl$Standard.format(FormatterStepImpl.java:82)                                                                                                                        
        at com.diffplug.spotless.FormatterStep$Strict.format(FormatterStep.java:103)                                                                                                                                 
        at com.diffplug.spotless.Formatter.compute(Formatter.java:246)                                                                                                                                               
        at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:203)                                                                                                                                 
        at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:190)                                                                                                                                 
        at com.diffplug.gradle.spotless.SpotlessTaskImpl.processInputFile(SpotlessTaskImpl.java:105)                                                                                                                 
        at com.diffplug.gradle.spotless.SpotlessTaskImpl.performAction(SpotlessTaskImpl.java:89)                                                                                                                     
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)                                                                                                     
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)                                                                                                                                                
        at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:125)                                                                                                                                        
        ...
nedtwigg commented 2 months ago

You can specify 24.2.0 yourself. I'd be okay with a PR that adds support for setting the version to *, but I want users to explicitly say "I don't care which version", because otherwise we get issues when the formatting is one way on dev machines but failing on CI because of a version difference.