Hi @Miezhiko!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

🔗 Helpful links

• 🧪  See artifacts and rendered test results at hud.pytorch.org/pr/70298
• 📄  Preview docs built from this PR
• 📄  Preview C++ docs built from this PR
• 🔧  Opt-in to CIFlow to control what jobs run on your PRs

💊 CI failures summary and remediations

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

• 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

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

linux-xenial-py3.6-clang7-asan / test (default, 1, 3, linux.2xlarge) (1/1)

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

2021-12-22T09:19:37.4857101Z     #9 0x55fb4f3b88f2 in PyEval_EvalCode /home/builder/ktietz/cos6/ci_cos6/python_1622833237666/work/Python/ceval.c:731
2021-12-22T09:19:37.4857832Z     #10 0x55fb4f420cd5 in run_mod /home/builder/ktietz/cos6/ci_cos6/python_1622833237666/work/Python/pythonrun.c:1025
2021-12-22T09:19:37.4858584Z     #11 0x55fb4f422d5d in PyRun_StringFlags /home/builder/ktietz/cos6/ci_cos6/python_1622833237666/work/Python/pythonrun.c:949
2021-12-22T09:19:37.4859429Z     #12 0x55fb4f422dbb in PyRun_SimpleStringFlags /home/builder/ktietz/cos6/ci_cos6/python_1622833237666/work/Python/pythonrun.c:445
2021-12-22T09:19:37.4860226Z     #13 0x55fb4f423926 in run_command /home/builder/ktietz/cos6/ci_cos6/python_1622833237666/work/Modules/main.c:301
2021-12-22T09:19:37.4860888Z     #14 0x55fb4f423926 in Py_Main /home/builder/ktietz/cos6/ci_cos6/python_1622833237666/work/Modules/main.c:749
2021-12-22T09:19:37.4861666Z     #15 0x55fb4f35d196 in main /home/builder/ktietz/cos6/ci_cos6/python_1622833237666/work/Programs/python.c:69
2021-12-22T09:19:37.5357429Z     #16 0x7fbbf308183f in __libc_start_main /build/glibc-S7Ft5T/glibc-2.23/csu/../csu/libc-start.c:291
2021-12-22T09:19:37.5358127Z     #17 0x55fb4f3ed33d in _start (/opt/conda/bin/python3.6+0x1a733d)
2021-12-22T09:19:37.5358463Z 
2021-12-22T09:19:37.5359295Z SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /var/lib/jenkins/workspace/aten/src/ATen/Utils.cpp:20:3 in 
2021-12-22T09:19:37.5542101Z + retcode=1
2021-12-22T09:19:37.5542914Z + set -e
2021-12-22T09:19:37.5543323Z + return 1
2021-12-22T09:19:37.5545455Z + [[ linux-xenial-py3.6-clang7-asan-default == *-NO_AVX-* ]]
2021-12-22T09:19:37.5546205Z + [[ default == \n\o\g\p\u\_\N\O\_\A\V\X ]]
2021-12-22T09:19:37.5546940Z + [[ linux-xenial-py3.6-clang7-asan-default == *-NO_AVX2-* ]]
2021-12-22T09:19:37.5547599Z + [[ default == \n\o\g\p\u\_\N\O\_\A\V\X\2 ]]
2021-12-22T09:19:37.5548552Z + [[ linux-xenial-py3.6-clang7-asan-default == *-NO_AVX512-* ]]
2021-12-22T09:19:37.5549276Z + [[ default == \n\o\g\p\u\_\N\O\_\A\V\X\5\1\2 ]]
2021-12-22T09:19:37.5550001Z + [[ linux-xenial-py3.6-clang7-asan-default == *tbb* ]]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@malfet added description to PR, it's still targeting the same fork, can't verify myself if patch was merged back to upstream because I'm not aware of that one but seems like fork is still maintained.

@malfet can you provide closure to this PR? The Archlinux packaging team would love for this to get merged in.

there used to be the patch on top of breakpad

Can you specify which particular patchset? Since the same fork is being used, I would assume the patch still exists in the fork.

Edit: I just checked and the fork is two commits ahead from google/breakpad. The latest commit being a merge from google/breakpad and the second to last commit being what I can only assume to be the patch you are talking about.

https://github.com/driazati/breakpad/blob/edbb99f95c75be27d038fffb1d969cdacf705db2/src/tools/mac/upload_system_symbols/go.mod

As we can see the latest commit still has that second to last change i.e. the merge from upstream in the fork has not affected the diverging commit (the patch in question) in the fork.

Therefore this should be safe to merge as it updates the breakpad repo in our pipeline while also maintaining the original patch we used the fork in the first place for.

@Miezhiko thanks for the PR btw!

@malfet sorry for the ping but if you get the time can you provide closure to this PR?

I believe we have answered your concerns regarding this change. Feel free to ask any other questions.

Pinging @seemethere for visibility

@Miezhiko can you add "Fixes #73113" in OP so that that issue can get closed with the merge of this PR.

@acxz done

@malfet @seemethere @suo who can we ping for a review of this PR?

Also is it usual for the CI to skip so many tests? Can someone restart the CI as well?

Thank you.

@acxz It looks like breakpad is not actively used by the community, so instead of updating it to a forked repos, I think it would be better to disable it altogether, though I hadn't had time to finish #75394 yet, let me give it one more try

breakpad is not actively used by the community

I see.

I think it would be better to disable it altogether

Got it, makes sense.

let me give it one more try

take your time, I think most users that are currently experiencing this issue are just patching around it themselves, so its prob not a big deal. However, I do ask that if it taking a long enough time, having this commit in the pytorch codebase wouldn't hurt either.

it is really simpler to remove breakpad than to merge this PR >__<

joking aside I don't think I needed breakpad myself, just cmake flag was ON by default and I expect that most of linux distributions packagers or source based linux users needed such patch for recent releases and it's always safer and simpler taking merged and tested patch than researching the problem and providing own solution for each.

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

Miezhiko looks like that breakpad has been removed. I think you can go ahead and close this PR and the associated issues with it.