Skip to content

Use python3.6 compatible APIs in clang_tidy.py #60659

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

Closed
wants to merge 2 commits into from

Conversation

1ntEgr8
Copy link
Contributor

@1ntEgr8 1ntEgr8 commented Jun 24, 2021

This PR make tools/clang_tidy.py use python 3.6 APIs for asyncio and shlex.

I ran into some issues when running this script with the -j flag inside of the clang-tidy docker image (which uses python 3.6). Specifically, the functions asycnio.run and shlex.join are available in python >= 3.8.

This change does not affect CI because we do not run the clang-tidy job in parallel.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 24, 2021

💊 CI failures summary and remediations

As of commit 48b5f9e (more details on the Dr. CI page and at hud.pytorch.org/pr/60659):


  • 2/2 failures possibly* introduced in this PR
    • 1/2 non-scanned failure(s)

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_linux_xenial_py3_clang5_asan_test2 (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Jun 25 00:50:20 RuntimeError: test_unary_ufuncs failed!
Jun 25 00:50:19     #172 0x557aa1e8c196 in main /home/builder/ktietz/cos6/ci_cos6/python_1622833237666/work/Programs/python.c:69
Jun 25 00:50:19     #173 0x7f2faa14083f in __libc_start_main /build/glibc-S7Ft5T/glibc-2.23/csu/../csu/libc-start.c:291
Jun 25 00:50:19     #174 0x557aa1f1c33d in _start (/opt/conda/bin/python3.6+0x1a733d)
Jun 25 00:50:19 
Jun 25 00:50:19 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /var/lib/jenkins/workspace/aten/src/ATen/native/Math.h:217:17 in 
Jun 25 00:50:20 Traceback (most recent call last):
Jun 25 00:50:20   File "test/run_test.py", line 1310, in <module>
Jun 25 00:50:20     main()
Jun 25 00:50:20   File "test/run_test.py", line 1289, in main
Jun 25 00:50:20     raise RuntimeError(err_message)
Jun 25 00:50:20 RuntimeError: test_unary_ufuncs failed!
Jun 25 00:50:20 + cleanup
Jun 25 00:50:20 + retcode=1
Jun 25 00:50:20 + set +x
Jun 25 00:50:20 =================== sccache compilation log ===================
Jun 25 00:50:21 =========== If your build fails, please take a look at the log above for possible reasons ===========
Jun 25 00:50:21 Compile requests                      2
Jun 25 00:50:21 Compile requests executed             0
Jun 25 00:50:21 Cache hits                            0
Jun 25 00:50:21 Cache misses                          0
Jun 25 00:50:21 Cache timeouts                        0

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@1ntEgr8 1ntEgr8 requested review from samestep and janeyx99 June 24, 2021 16:06
Copy link
Contributor

@janeyx99 janeyx99 left a comment

Choose a reason for hiding this comment

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

Looks fine, but would there be a downside to upgrading the docker image's python?

@1ntEgr8
Copy link
Contributor Author

1ntEgr8 commented Jun 24, 2021

@janeyx99 There's no major downside in upgrading the Docker image's python version. The image takes quite some time to build. I thought updating the script itself would be faster. The modified script can also run with python >= 3.8, so changing the docker python version should not affect it.

@driazati
Copy link
Contributor

driazati commented Jun 24, 2021

This change does not affect CI because we do not run the clang-tidy job in parallel.

Later on down the road maybe this should change since clang-tidy does take a while compared to other lint jobs, we just have to be careful about making sure the output streams don’t get interleaved.

@1ntEgr8 1ntEgr8 force-pushed the switch-to-py3.6-ct-script branch from 0d8e7a9 to 48b5f9e Compare June 24, 2021 23:01
@facebook-github-bot
Copy link
Contributor

@1ntEgr8 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@1ntEgr8 merged this pull request in 8216da1.

asuhan pushed a commit to asuhan/pytorch that referenced this pull request Jun 28, 2021
Summary:
This PR make `tools/clang_tidy.py` use python 3.6 APIs for `asyncio` and `shlex`.

I ran into some issues when running this script with the `-j` flag inside of the clang-tidy docker image (which uses python 3.6). Specifically, the functions `asycnio.run` and `shlex.join` are available in python >= 3.8.

This change does not affect CI because we do not run the clang-tidy job in parallel.

Pull Request resolved: pytorch#60659

Reviewed By: albanD

Differential Revision: D29377851

Pulled By: 1ntEgr8

fbshipit-source-id: 92ab7ee6782b78d40ffccd03f1718ede4204d948
asuhan pushed a commit that referenced this pull request Jun 30, 2021
Summary:
This PR make `tools/clang_tidy.py` use python 3.6 APIs for `asyncio` and `shlex`.

I ran into some issues when running this script with the `-j` flag inside of the clang-tidy docker image (which uses python 3.6). Specifically, the functions `asycnio.run` and `shlex.join` are available in python >= 3.8.

This change does not affect CI because we do not run the clang-tidy job in parallel.

Pull Request resolved: #60659

Reviewed By: albanD

Differential Revision: D29377851

Pulled By: 1ntEgr8

fbshipit-source-id: 92ab7ee6782b78d40ffccd03f1718ede4204d948
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants