[Enhancement] Fix collect_env on Windows #1789

SuTanTank · 2022-03-10T04:50:44Z

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

collect_env() use tail and head which is not available on most Windows machines.

'tail' is not recognized as an internal or external command, operable program or batch file.
'head' is not recognized as an internal or external command, operable program or batch file.

Modification

Remove the tail and head, and use string find to extract env infos.

BC-breaking (Optional)

No

Use cases (Optional)

Checklist

Before PR:

I have read and followed the workflow indicated in the CONTRIBUTING.md to create this PR.
Pre-commit or linting tools indicated in CONTRIBUTING.md are used to fix the potential lint issues.
Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
New functionalities are covered by complete unit tests. If not, please add more unit test to ensure the correctness.
The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

If the modification has potential influence on downstream or other related projects, this PR should be tested with some of those projects, like MMDet or MMCls.
CLA has been signed and all committers have signed the CLA in this PR.

teamwong111

LGTM

HAOCHENYE · 2022-03-10T08:08:31Z

mmcv/utils/env.py

        gcc = gcc.decode('utf-8').strip()
+        first_line = gcc.find('\n')
+        gcc = gcc[:first_line].strip()


Hi, thanks for your contribution. mmcv compiled with cl.exe in windows, it could be better if collect_env can output the
information of 'cl.exe'

Usually there are multiple cl.exe on one machine (multiple VS version and cross-compliation tools) and cl is not avaliable in common shell environment. So it would be not an easy task and might even output misleading information.

The main reason I need to patch this is, when I use mmcv lite version on Windows, it complains that no CUDA runtime is found, probably because of the missing tail and head.

But on windows, mmcv does not compiled with gcc, so it is confusing if it happens to output mingw gcc information. Should we choose how to export gcc information via sys.platform? We can make env_info['GCC'] = 'n/a' as before. Besides, how does “no CUDA runtime is found” occurs?

HAOCHENYE · 2022-03-10T08:09:47Z

mmcv/utils/env.py

@@ -56,16 +56,20 @@ def collect_env():
        if CUDA_HOME is not None and osp.isdir(CUDA_HOME):
            try:
                nvcc = osp.join(CUDA_HOME, 'bin/nvcc')
-                nvcc = subprocess.check_output(
-                    f'"{nvcc}" -V | tail -n1', shell=True)
+                nvcc = subprocess.check_output(f'"{nvcc}" -V', shell=True)


It could be better if we can get the last line of nvcc -V which is consistent with the former version.

On Linux

nvcc: NVIDIA (R) Cuda compiler driver Copyright (c) 2005-2019 NVIDIA Corporation Built on Wed_Oct_23_19:24:38_PDT_2019 Cuda compilation tools, release 10.2, V10.2.89

On Windows (11.5)

nvcc: NVIDIA (R) Cuda compiler driver Copyright (c) 2005-2021 NVIDIA Corporation Built on Thu_Nov_18_09:52:33_Pacific_Standard_Time_2021 Cuda compilation tools, release 11.5, V11.5.119 Build cuda_11.5.r11.5/compiler.30672275_0

Not sure what it looks like on other Windows machines. but the last line on Linux match with the second last line on Windows.

I will add 'Cuda compilation tools' to the output.

OK~ thanks for your explanation.

May leave comments in the code to explain.

imabackstabber

LGTM

SuTanTank · 2022-03-12T05:07:24Z

Since gcc is not always the compiler. I changed the key to CC. On windows, it returns the first line of copyright banner of cl.exe, like

Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30140 for x64

mmcv/utils/env.py

tests/test_utils/test_env.py

zhouzaida

UnicodeDecodeError was raised on my computer.

>>> from mmcv.utils import collect_env
>>> collect_env()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "D:\codebases\mmcv\mmcv\utils\env.py", line 89, in collect_env
    env_info['MSVC'] = cc.decode('utf-8').partition('\n')[0].strip()
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xd3 in position 0: invalid continuation byte

mmcv/utils/env.py

zhouzaida · 2022-04-04T15:12:51Z

mmcv/utils/env.py

+            encoding = locale.getdefaultlocale()[1]
+            env_info['MSVC'] = cc.decode(encoding).partition('\n')[0].strip()
+            env_info['GCC'] = 'n/a'
+            del ccompiler


Is it necessary to manually delete ccompiler?

Maybe not. I did this because that initialize() call took a few seconds, so I thought this object may take some unnecessary memory.

It will be released by gc in later so we can remove this line.

mmcv/utils/env.py

mm-assistant bot added the size/XS label Mar 10, 2022

[fix] fix collect_env() on Windows

65975d7

SuTanTank force-pushed the fix/envs branch from e73e27a to 65975d7 Compare March 10, 2022 07:10

zhouzaida requested review from HAOCHENYE, imabackstabber and teamwong111 March 10, 2022 07:33

teamwong111 approved these changes Mar 10, 2022

View reviewed changes

HAOCHENYE reviewed Mar 10, 2022

View reviewed changes

fix(utils): add back 'Cuda compilation tools' to nvcc info

746fecb

imabackstabber approved these changes Mar 10, 2022

View reviewed changes

HAOCHENYE added the Windows label Mar 11, 2022

feat(utils): find default ccompiler

9d57970

SuTanTank force-pushed the fix/envs branch from 53904ce to 9d57970 Compare March 12, 2022 05:00

SuTanTank added 2 commits March 12, 2022 16:11

fix(utils, test): parse cl.exe version instead of path

29692f9

fix: code style

72666bb

HAOCHENYE reviewed Mar 12, 2022

View reviewed changes

mmcv/utils/env.py Show resolved Hide resolved

fix(tools): use distutils

f29bb33

SuTanTank changed the title ~~[fix] fix collect_env() on Windows~~ [feature] make collect_env() works on Windows Mar 14, 2022

SuTanTank added 2 commits March 15, 2022 13:12

[fix] collect_env() returns GCC or MSVC

6363b09

[fix] unitest for collect_env()

3ebdefb

HAOCHENYE approved these changes Mar 16, 2022

View reviewed changes

zhouzaida reviewed Mar 17, 2022

View reviewed changes

mmcv/utils/env.py Outdated Show resolved Hide resolved

add comments

ecd4e72

zhouzaida reviewed Mar 20, 2022

View reviewed changes

tests/test_utils/test_env.py Outdated Show resolved Hide resolved

test: add GCC to expected_keys

433cbe7

zhouzaida approved these changes Mar 21, 2022

View reviewed changes

zhouzaida changed the title ~~[feature] make collect_env() works on Windows~~ [feature] Fix collect_env() on Windows Apr 2, 2022

zhouzaida changed the title ~~[feature] Fix collect_env() on Windows~~ [feature] Fix collect_env on Windows Apr 2, 2022

zhouzaida changed the title ~~[feature] Fix collect_env on Windows~~ [Enhancement] Fix collect_env on Windows Apr 2, 2022

zhouzaida self-requested a review April 2, 2022 10:59

zhouzaida requested changes Apr 2, 2022

View reviewed changes

zhouzaida reviewed Apr 2, 2022

View reviewed changes

mmcv/utils/env.py Outdated Show resolved Hide resolved

zhouzaida mentioned this pull request Apr 4, 2022

Support collect_env on Windows #1539

Closed

fix: code style

f59c2b3

SuTanTank force-pushed the fix/envs branch from fdc9b9b to f59c2b3 Compare April 4, 2022 12:25

zhouzaida reviewed Apr 4, 2022

View reviewed changes

SuTanTank commented Apr 5, 2022

View reviewed changes

mmcv/utils/env.py Outdated Show resolved Hide resolved

Update mmcv/utils/env.py

3fe1bfd

zhouzaida approved these changes Apr 5, 2022

View reviewed changes

zhouzaida merged commit c33f248 into open-mmlab:master Apr 5, 2022

SuTanTank deleted the fix/envs branch April 6, 2022 06:50

teamwong111 mentioned this pull request Apr 8, 2022

ImportError: DLL load failed #1862

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] Fix collect_env on Windows #1789

[Enhancement] Fix collect_env on Windows #1789

SuTanTank commented Mar 10, 2022

teamwong111 left a comment

HAOCHENYE Mar 10, 2022

SuTanTank Mar 10, 2022

HAOCHENYE Mar 11, 2022 •

edited

Loading

HAOCHENYE Mar 10, 2022

SuTanTank Mar 10, 2022

HAOCHENYE Mar 11, 2022

ZwwWayne Mar 18, 2022

imabackstabber left a comment

SuTanTank commented Mar 12, 2022 •

edited

Loading

zhouzaida left a comment

zhouzaida Apr 4, 2022

SuTanTank Apr 4, 2022

zhouzaida Apr 4, 2022 •

edited

Loading

[Enhancement] Fix collect_env on Windows #1789

[Enhancement] Fix collect_env on Windows #1789

Conversation

SuTanTank commented Mar 10, 2022

Motivation

Modification

BC-breaking (Optional)

Use cases (Optional)

Checklist

teamwong111 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HAOCHENYE Mar 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imabackstabber left a comment

Choose a reason for hiding this comment

SuTanTank commented Mar 12, 2022 • edited Loading

zhouzaida left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhouzaida Apr 4, 2022 • edited Loading

Choose a reason for hiding this comment

HAOCHENYE Mar 11, 2022 •

edited

Loading

SuTanTank commented Mar 12, 2022 •

edited

Loading

zhouzaida Apr 4, 2022 •

edited

Loading