-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Black formatting is not applied (but black runs) #18561
Comments
Exactly the same issue here. Only Windows not working. I am also using a global black by the way. Also reference to the StackOverflow question |
Some further testing from my end seems to point that somehow it seems vscode-python does not pay well with black installed through pipx on windows, anymore. A few things I have trialed: |
@hcoohb We have not changed anything in this area in the recently. If you revert to a python extension version older than 2021.12.* does it work? |
@karthiknadig Installing python extension 2021.11.* did not change anything indeed, still not working with the pipx install. I am sure it was working last month though... Could a Vscode update have any impact? |
@hcoohb It looks like we are not getting anything in stdout, when we run black.exe installed via We are planning on moving formatting into their own extension to make it easier for tools and community to investigate and fix bugs. If you want to try out the prototype, please see instructions here: psf/black#2883 |
pipx also sets up a dedicated venv, it is weird why this works but pipx doesn't. Maybe we can manually set up a venv under pipx's location. I will try if I find a time. |
Same problem here. I have black 22.1.0 installed in a virtualenv created by poetry. Black formats correctly the files of my project, when I run it from the command line using:
but if I try to run it from vscode, it will add two options |
I did some investigating, and the TL;DR is that this isn't a bug in either VSCode or Black, and technically not even in pipx. Rather, this is a bug in a component of distlib, which is used to create the I'll open an issue there in a bit, but I just wanted to share the whole story here, as it's quite beautiful 🥲. Update 2022-02-28: Issue opened. Note: I'll be using Python 3.10 in the analysis below, as that's what I have, but the general ideas should apply to other versions as well. Chapter 1: Leaving A Comment Doesn't Make It SoIf we look at the exit code of Lucky for us, the Python devs provide private debugging symbols, so we can recover the full call stack with line numbers:
The The Sidenote about fds: the Microsoft C Runtime "emulates" POSIX-like file descriptors so that functions like Back to the story. In order for Furthermore, the CRT has a bug in the duplication code: bool success = false;
__try
{
BOOL const result = DuplicateHandle(...);
if (!result)
{
...
new_fh = -1;
__leave;
}
...
}
__finally
{
if (!success)
{
#pragma warning(disable:__WARNING_BUFFER_UNDERFLOW) // 26001 new_fh can't be -1 here
_osfile(new_fh) &= ~FOPEN;
}
...
} Narrator voice: If Note: We're talking about version 10.0.19041.0 of the UCRT. AFAICT this has been fixed in newer versions. However, the version you have depends on Windows Update, so you may be out of luck. Chapter 2: Left Out Of The InheritanceAlright, so this explains why Black doesn't return anything, as it just crashes during the Python interpreter's initialization. But how come fd 2 is associated with a nonexistent handle? To understand that, we need to look into how the standard I/O fds are initialized at process startup. The initialization flow is described rather well in the code itself. For our analysis the important part is this: on startup, the CRT obtains the Crucially, the CRT does try to validate each handle in the Now, if we look at the handle table for Except, the stderr handle is not inheritable! Something in Indeed, setting a breakpoint on But what is For our purposes, however, the only interesting part is this: memset(&si, 0, sizeof(si));
GetStartupInfoW(&si);
/*
* See https://github.com/pypa/pip/issues/10444#issuecomment-973396812
*/
if ((si.dwFlags & (STARTF_USEHOTKEY | STARTF_UNDOC_MONITOR)) == 0) {
HANDLE hIn = GetStdHandle(STD_INPUT_HANDLE);
HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE);
HANDLE hErr = GetStdHandle(STD_ERROR_HANDLE);
ok = safe_duplicate_handle(hIn, &si.hStdInput);
assert(ok, "stdin duplication failed");
CloseHandle(hIn);
ok = safe_duplicate_handle(hOut, &si.hStdOutput);
assert(ok, "stdout duplication failed");
CloseHandle(hOut);
/* We might need stderr late, so don't close it but mark as non-inheritable */
SetHandleInformation(hErr, HANDLE_FLAG_INHERIT, 0);
ok = safe_duplicate_handle(hErr, &si.hStdError);
assert(ok, "stderr duplication failed");
si.dwFlags |= STARTF_USESTDHANDLES;
}
ok = CreateProcessW(NULL, cmdline, NULL, NULL, TRUE, 0, NULL, NULL, &si, &child_process_info); This does several things:
However, the original There's one other thing of note here: in the code above, each I/O handle is duplicated and then closed. Due to how Windows reuses handle values, this means that the next created handle will probably get the same value as the one that was just closed. So after running the code above we'll most likely have:3
This means that when the CRT goes to initialize its fds in the child process, stdin and stdout will have valid handles, just that they'd refer to the wrong objects. And, as mentioned previously, stderr will have a garbage handle value. Chapter 3: A Series of Unfortunate EventsAt this point we have enough information to piece together the series of events leading to the crash:
We can now also explain why Black works when run from the command-line. And that's about it. A story of undocumented structures, random chance, and ABIs you didn't know you had to uphold. Truly beautiful. Appendix A: All My SonsActually, that's not everything. There's one other process in play when running Black. The full tree looks like this:
Why is this interesting? Because this launcher also performs the handle duplication dance. Except, in this case it doesn't close the original handles. Since the originals were inherited from So the analysis above is still valid, we just end up with a couple more duplicate handles. Appendix B: Well-Known SecretsRecall how VSCode launches I found only one: Upon further examination, it appears that VSCode uses libuv for launching the process. And look what I found there: startup.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW;
startup.cbReserved2 = uv__stdio_size(process->child_stdio_buffer);
startup.lpReserved2 = (BYTE*) process->child_stdio_buffer;
startup.hStdInput = uv__stdio_handle(process->child_stdio_buffer, 0);
startup.hStdOutput = uv__stdio_handle(process->child_stdio_buffer, 1);
startup.hStdError = uv__stdio_handle(process->child_stdio_buffer, 2); It would appear that the Appendix C: More FunHere's a bit of C code that launches the command it receives in its arguments (just don't put spaces in any of them): #include <process.h>
int wmain(int argc, wchar_t ** argv)
{
return _wspawnvp(_P_WAIT, argv[1], argv + 1);
} This isn't doing anything even remotely interesting, yet when you run
Debugging this is left as an exercise for the reader 😎. Footnotes
|
It turns out manually created venv under UPDATE Yes, there are differences. The Normal venv with pip inside, works fine
Thin venv without pip, but reference to pip outside, buggy. This is how pipx works
What's more, So the current workaround is to create a dedicated venv and install black in it, then point |
@frostming What's the pip version in the "fine" venv? The buggy launcher was added in pip 22.0. |
Oh, I didn't notice that! so the root cause is |
I can confirm that it's a pip problem. I downgraded pip from 22.0.3 to 21.3.1 and reinstalled black 22.1 and everything started to work again (I've done everything inside a venv). |
Could anyone test a fixed version of the launchers? https://bitbucket.org/mbikovitsky/simple_launcher/downloads/handle-dup-fix.zip
The updated code from which these were built is here. |
I can confirm it fixes the problem and works like a charm. |
Closing this here since this is not caused by the python extension. |
Issue Type: Bug
Behaviour
When formatting a *.py file with black, no changes are applied.
I can see in the log black being run without error but no changes are applied to my file.
Output of python after formatting:
when I run that same command in terminal, I can see the output of black which seems correct:
I am using the following settings:
(black is installed through pipx)
Running
~\.local\bin\black.exe .\test.py
does format my file correctly.Is it because of the
--diff
arg vscode give to black, how to remove it?This setup was working last month for sure but now something seems to have changed and cannot figure out... I am so confused
Thanks for the help
Diagnostic data
python.languageServer
setting: PylanceUser Settings
Extension version: 2022.0.1814523869
VS Code version: Code 1.64.2 (f80445acd5a3dadef24aa209168452a3d97cc326, 2022-02-09T22:02:28.252Z)
OS version: Windows_NT x64 10.0.19042
Restricted Mode: No
System Info
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
skia_renderer: enabled_on
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
A/B Experiments
The text was updated successfully, but these errors were encountered: