Skip to content

Commit

Permalink
roll back black for now due to Windows breakage
Browse files Browse the repository at this point in the history
  • Loading branch information
@dae
dae committed May 7, 2021
1 parent dfba66f commit d797900
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion pip/requirements.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ attrs==21.1.0
# pytest
beautifulsoup4==4.9.3
# via -r requirements.in
black==21.5b0
black==20.8b1
# via -r requirements.in
certifi==2020.12.5
# via requests
Expand Down

6 comments on commit d797900

@dae
Copy link
Member Author

@dae dae commented on d797900 May 7, 2021

Choose a reason for hiding this comment

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

@RumovZ are you interested in digging into this a bit? We could start by confirming it works correctly in a Python venv outside of Bazel, then compare what HOME/USERPROFILE/etc are set to when running run_format.py to see if the difference becomes apparent.

@RumovZ
Copy link
Collaborator

@RumovZ RumovZ commented on d797900 May 7, 2021

Choose a reason for hiding this comment

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

Will do!

@RumovZ
Copy link
Collaborator

@RumovZ RumovZ commented on d797900 May 11, 2021

Choose a reason for hiding this comment

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

When I run black outside bazel, $HOMEPATH and $USERPROFILE are set. These are the variables pathlib is looking for when determining the home directory. With bazel, only $HOME is set. Of course, this doesn't has to mean anything as it works fine with earlier black versions.
I don't seem to get anywhere here. Do you have a suggestion what I could try to find the issue?

@dae
Copy link
Member Author

@dae dae commented on d797900 May 11, 2021

Choose a reason for hiding this comment

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

Does explicitly setting them in the environment of run_format.py (via env= in subprocess.run) help at all? If so, we may have a way to work around it. You could also try running bazel build with --action_env=HOMEPATH --action_env=USERPROFILE to see if that makes any difference. If neither of those help and you don't have any other quick things to test, we can just drop it for now and hope it gets resolved in black in the mean time

@RumovZ
Copy link
Collaborator

@RumovZ RumovZ commented on d797900 May 12, 2021

Choose a reason for hiding this comment

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

I tried variations of .\run --action_env=USERPROFILE="C:\\msys64\\home\\Christian" with no luck. But maybe I didn't try the right syntax for Powershell.
Setting the variable in the script works fine, on the other hand. But note that the problem is not with the call in run_format.py but in hookslib.py.

diff --git a/pip/requirements.txt b/pip/requirements.txt
index 3e89afdf..bc2ace9e 100644
--- a/pip/requirements.txt
+++ b/pip/requirements.txt
@@ -12,7 +12,7 @@ attrs==21.1.0
     #   pytest
 beautifulsoup4==4.9.3
     # via -r requirements.in
-black==20.8b1
+black==21.5b0
     # via -r requirements.in
 certifi==2020.12.5
     # via requests
diff --git a/pylib/tools/hookslib.py b/pylib/tools/hookslib.py
index f8262aa9..b05558ae 100644
--- a/pylib/tools/hookslib.py
+++ b/pylib/tools/hookslib.py
@@ -161,6 +161,8 @@ def write_file(path: str, hooks: List[Hook], prefix: str, suffix: str):

     code += "\n" + suffix

+    import os
+    os.environ["USERPROFILE"] = os.environ["HOME"]
     with open(path, "wb") as file:
         file.write(code.encode("utf8"))
     subprocess.run([sys.executable, "-m", "black", "-q", path], check=True)

@dae
Copy link
Member Author

@dae dae commented on d797900 May 13, 2021

Choose a reason for hiding this comment

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

Thanks for looking into it, that seems to have helped. I had to add the same to the two run_formats as well, and gate it to Windows, but it all seems to be working now

Please sign in to comment.