Skip to content

Conversation

@samestep
Copy link
Contributor

@samestep samestep commented Mar 5, 2021

Context: #53299 (comment)

These are the only hand-written parts of this diff:

  • the addition to .github/workflows/lint.yml
  • the file endings changed in these four files (to appease FB-internal land-blocking lints):
    • GLOSSARY.md
    • aten/src/ATen/core/op_registration/README.md
    • scripts/README.md
    • torch/csrc/jit/codegen/fuser/README.md

The rest was generated by running this command (on macOS):

git grep -I -l ' $' -- . ':(exclude)**/contrib/**' ':(exclude)third_party' | xargs gsed -i 's/ *$//'

I looked over the auto-generated changes and didn't see anything that looked problematic.

Test plan:

This run (after adding the lint but before removing existing trailing spaces) failed:

This run (on the tip of this PR) succeeded:

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 5, 2021

💊 CI failures summary and remediations

As of commit 6f1c30e (more details on the Dr. CI page):


  • 2/2 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

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

See GitHub Actions build clang-tidy (1/1)

Step: "Add annotations" (full log | diagnosis details | 🔁 rerun)

2021-03-05T22:03:54.1693253Z update-alternatives: error: no alternatives for nsight-sys
2021-03-05T22:03:54.0977394Z Setting up x11proto-damage-dev (1:2018.4-4) ...
2021-03-05T22:03:54.1018682Z Setting up cuda-nvjpeg-10-2 (10.2.89-1) ...
2021-03-05T22:03:54.1120628Z Setting up libxcb-dri3-dev:amd64 (1.13-2~ubuntu18.04) ...
2021-03-05T22:03:54.1161868Z Setting up libcublas10 (10.2.3.254-1) ...
2021-03-05T22:03:54.1207940Z Setting up libcublas-dev (10.2.3.254-1) ...
2021-03-05T22:03:54.1269354Z Setting up cuda-cufft-10-2 (10.2.89-1) ...
2021-03-05T22:03:54.1419712Z Setting up cuda-nsight-compute-10-2 (10.2.89-1) ...
2021-03-05T22:03:54.1468679Z Setting up nsight-systems-2020.4.3 (2020.4.3.7-10543b6) ...
2021-03-05T22:03:54.1593291Z update-alternatives: using /opt/nvidia/nsight-systems/2020.4.3/target-linux-x64/nsys to provide /usr/local/bin/nsys (nsys) in auto mode
2021-03-05T22:03:54.1682446Z update-alternatives: error: alternative path /opt/nvidia/nsight-systems/2020.4.3/host-linux-x64/nsight-sys doesn't exist
2021-03-05T22:03:54.1693253Z update-alternatives: error: no alternatives for nsight-sys
2021-03-05T22:03:54.1758221Z update-alternatives: using /opt/nvidia/nsight-systems/2020.4.3/host-linux-x64/nsys-ui to provide /usr/local/bin/nsys-ui (nsys-ui) in auto mode
2021-03-05T22:03:54.1840495Z Setting up libxcb-shape0-dev:amd64 (1.13-2~ubuntu18.04) ...
2021-03-05T22:03:54.1905679Z Setting up cuda-cusparse-10-2 (10.2.89-1) ...
2021-03-05T22:03:54.2016447Z Setting up cuda-cuobjdump-10-2 (10.2.89-1) ...
2021-03-05T22:03:54.2066446Z Setting up libxcb-glx0-dev:amd64 (1.13-2~ubuntu18.04) ...
2021-03-05T22:03:54.2117577Z Setting up libgles2:amd64 (1.0.0-2ubuntu2.3) ...
2021-03-05T22:03:54.2169518Z Setting up libglu1-mesa:amd64 (9.0.0-2.1build1) ...
2021-03-05T22:03:54.2236144Z Setting up libegl-mesa0:amd64 (20.0.8-0ubuntu1~18.04.1) ...
2021-03-05T22:03:54.2422690Z Setting up cuda-sanitizer-api-10-2 (10.2.89-1) ...
2021-03-05T22:03:54.2542830Z Setting up cuda-nvjpeg-dev-10-2 (10.2.89-1) ...

1 failure not recognized by patterns:

Job Step Action
CircleCI pytorch_linux_xenial_py3_6_gcc5_4_test Report results 🔁 rerun

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.

@samestep samestep marked this pull request as ready for review March 5, 2021 21:12
@samestep samestep requested a review from a team March 5, 2021 21:12
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@facebook-github-bot
Copy link
Contributor

@samestep merged this pull request in 8c798e0.

@ngimel
Copy link
Collaborator

ngimel commented Mar 8, 2021

Broke distributed tests, see e.g. https://app.circleci.com/pipelines/github/pytorch/pytorch/281903/workflows/fa16bc70-463b-40c0-8888-7bd25533c8dd/jobs/11357103. Distributed failures are also in this PR's CI.

@zhangguanheng66
Copy link
Contributor

This PR just cleans up the trailing whitespace so I don't understand how it can break the master branch. However, I will revert it to recover the master branch.

@samestep samestep deleted the lint-trailing-whitespace branch March 25, 2021 18:54
facebook-github-bot pushed a commit that referenced this pull request Mar 30, 2021
Summary:
*Context:* #53406 added a lint for trailing whitespace at the ends of lines. However, in order to pass FB-internal lints, that PR also had to normalize the trailing newlines in four of the files it touched. This PR adds an OSS lint to normalize trailing newlines.

The changes to the following files (made in 54847d0) are the only manually-written parts of this PR:

- `.github/workflows/lint.yml`
- `mypy-strict.ini`
- `tools/README.md`
- `tools/test/test_trailing_newlines.py`
- `tools/trailing_newlines.py`

I would have liked to make this just a shell one-liner like the other three similar lints, but nothing I could find quite fit the bill. Specifically, all the answers I tried from the following Stack Overflow questions were far too slow (at least a minute and a half to run on this entire repository):

- [How to detect file ends in newline?](https://stackoverflow.com/q/38746)
- [How do I find files that do not end with a newline/linefeed?](https://stackoverflow.com/q/4631068)
- [How to list all files in the Git index without newline at end of file](https://stackoverflow.com/q/27624800)
- [Linux - check if there is an empty line at the end of a file [duplicate]](https://stackoverflow.com/q/34943632)
- [git ensure newline at end of each file](https://stackoverflow.com/q/57770972)

To avoid giving false positives during the few days after this PR is merged, we should probably only merge it after #54967.

Pull Request resolved: #54737

Test Plan:
Running the shell script from the "Ensure correct trailing newlines" step in the `quick-checks` job of `.github/workflows/lint.yml` should print no output and exit in a fraction of a second with a status of 0. That was not the case prior to this PR, as shown by this failing GHA workflow run on an earlier draft of this PR:

- https://github.com/pytorch/pytorch/runs/2197446987?check_suite_focus=true

In contrast, this run (after correcting the trailing newlines in this PR) succeeded:

- https://github.com/pytorch/pytorch/pull/54737/checks?check_run_id=2197553241

To unit-test `tools/trailing_newlines.py` itself (this is run as part of our "Test tools" GitHub Actions workflow):
```
python tools/test/test_trailing_newlines.py
```

Reviewed By: malfet

Differential Revision: D27409736

Pulled By: samestep

fbshipit-source-id: 46f565227046b39f68349bbd5633105b2d2e9b19
xsacha pushed a commit to xsacha/pytorch that referenced this pull request Mar 31, 2021
Summary:
Context: pytorch#53299 (comment)

These are the only hand-written parts of this diff:
- the addition to `.github/workflows/lint.yml`
- the file endings changed in these four files (to appease FB-internal land-blocking lints):
  - `GLOSSARY.md`
  - `aten/src/ATen/core/op_registration/README.md`
  - `scripts/README.md`
  - `torch/csrc/jit/codegen/fuser/README.md`

The rest was generated by running this command (on macOS):
```
git grep -I -l ' $' -- . ':(exclude)**/contrib/**' ':(exclude)third_party' | xargs gsed -i 's/ *$//'
```

I looked over the auto-generated changes and didn't see anything that looked problematic.

Pull Request resolved: pytorch#53406

Test Plan:
This run (after adding the lint but before removing existing trailing spaces) failed:
- https://github.com/pytorch/pytorch/runs/2043032377

This run (on the tip of this PR) succeeded:
- https://github.com/pytorch/pytorch/runs/2043296348

Reviewed By: walterddr, seemethere

Differential Revision: D26856620

Pulled By: samestep

fbshipit-source-id: 3f0de7f7c2e4b0f1c089eac9b5085a58dd7e0d97
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.

6 participants