Skip to content

Disable -Werror on macOS test build#11090

Closed
apaszke wants to merge 1 commit intopytorch:masterfrom
apaszke:disable_mac_werror
Closed

Disable -Werror on macOS test build#11090
apaszke wants to merge 1 commit intopytorch:masterfrom
apaszke:disable_mac_werror

Conversation

@apaszke
Copy link
Contributor

@apaszke apaszke commented Aug 30, 2018

Don't get me wrong, -Werror is great. However, when you have to resubmit
your PR 3 times, because -Werror doesn't report half of those errors in
a Linux environment, it starts to get really annoying.
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.

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

@goldsborough
Copy link
Contributor

Why? I think it warrants an explanation 😅

@apaszke
Copy link
Contributor Author

apaszke commented Aug 30, 2018

Because I just spent over 2h waiting for 2 CI runs (and it's not the first time I had that problem). Both failed only because of -Werror on mac build. I think -Werror is great, but there are a couple problems with our settings right now, which makes working with it terribly annoying:

  • Those errors are irreproducible in a Linux environment. I tried building with WERROR=1 locally and everything was fine, but CI has still failed.
  • The CI build fails at the first file that triggers an error. This means that unless you have a macOS env ready for debugging, you'll need as many CI runs as you have files with warnings to fix the issue.
  • The -Werror build is part of the test phase, which means you need to wait over 30 mins to actually see that it's failing. It should happen in the build phase.

I'd be very happy to reenable this check, but we need to make sure we can reproduce all the failures locally without having to build PyTorch from scratch on a 4 core laptop.

@ezyang
Copy link
Contributor

ezyang commented Aug 30, 2018

(Just a debugging communication note: @goldsborough probably hasn't seen the internal discussion about this in OSS CI yet)

PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
Summary:
cc goldsborough
Pull Request resolved: pytorch#11090

Reviewed By: soumith

Differential Revision: D9582525

Pulled By: apaszke

fbshipit-source-id: 5d2c6e930e7b09f0ed5a35fbf4fe36b8845a2580
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.

5 participants