Use `@pytorch//` in bazel build files #89660

vors · 2022-11-24T22:10:05Z

This change aims to make bazel build more embeeding-friendly.
Namely, when PyTorch is included as an external repo in another project, it is usually included like this

        native.local_repository(
            name = "pytorch",
            path = ...,
            repo_mapping = repo_mapping,
        )

Or

        http_archive(
            name = "pytorch",
            urls = ...
            repo_mapping = repo_mapping,
        )

In this case, references to @// would resolve to the top-level WORKSPACE that includes PyTorch.
That makes upgrades harder because we need to carry around this patch.
Note that under some edge-case circumstances even // resolves to the top-level WORKSPACE.

This change makes the embedding of the bazel build easier without compromising anything for the main repo, since the @pytorch// still refers to the same thing.

cc @gujinghui @PenghuiCheng @XiaobingSuper @jianyuh @VitalyFedyunin @jgong5 @mingfeima @sanchitintel @ashokei @jingxu10 @min-jean-cho @yanbing-j @Guobing-Chen @Xia-Weiwen

pytorch-bot · 2022-11-24T22:10:07Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/89660

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 Failures

As of commit cc7bea3:

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

linux-foundation-easycla · 2022-11-24T22:10:09Z

The committers listed above are authorized under a signed CLA.

✅ login: vors / name: Sergei Vorobev (eed1bc4, cc7bea3)

vors · 2022-11-24T22:11:03Z

I didn't add the cc -- seems like some automation?

kit1980 · 2022-11-27T05:55:56Z

/easycla

kit1980 · 2022-11-27T21:04:05Z

@vors Could you check that the email on your commits listed in your GitHub account? This seems to be an issue that upsets EasyCLA.

vors · 2022-11-27T21:08:16Z

@kit1980 thank you! I plan to do more contributions as part of my work at Cruise, as well as help my teammates to do the same. Since PyTorch now part of Linux Foundation it seems like CLA also changed? Let me spend a little bit of time sorting out CLA with our legal team.

kit1980 · 2022-11-28T22:16:37Z

@vors besides signing the CLA, there is a technical CLA-check related detail: email on the commit (as seen by git) must be listed in your GitHub account, this seems to be not the case currently.

vors · 2022-11-29T06:05:33Z

Oh I see, let me fix it :)

vors · 2022-12-13T19:47:32Z

Ok I worked with our legal, we signed CLA and I'm now CLA manager for Cruise.

This should be ready to merge from the legal point on view :)

@kit1980 can you help with the code review?

kit1980 · 2022-12-15T08:29:19Z

/easycla

vors · 2022-12-20T21:13:51Z

@pytorchbot /rebase

pytorch-bot · 2022-12-20T21:13:52Z

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: argument command: invalid choice: '/rebase' (choose from 'merge', 'revert', 'rebase', 'label', 'drci')

usage: @pytorchbot [-h] {merge,revert,rebase,label,drci} ...

Try @pytorchbot --help for more info.

vors · 2022-12-20T21:14:08Z

@pytorchbot rebase

pytorch-bot · 2022-12-20T21:14:11Z

You don't have permissions to rebase this PR, only people with write permissions may rebase PRs.

sanchitintel · 2022-12-20T21:15:26Z

@pytorchbot rebase -b master

pytorchmergebot · 2022-12-20T21:17:10Z

@pytorchbot successfully started a rebase job. Check the current status here

pytorchmergebot · 2022-12-20T21:17:16Z

Successfully rebased sergei/master/add-pytorch onto refs/remotes/origin/master, please pull locally before adding more changes (for example, via git checkout sergei/master/add-pytorch && git pull --rebase)

kit1980 · 2022-12-20T21:25:12Z

/easycla

vors · 2022-12-20T21:42:20Z

/easycla

facebook-github-bot · 2022-12-20T21:45:31Z

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

vors · 2022-12-21T18:10:25Z

Gentle ping on this one, can we start getting a review?

kit1980 · 2022-12-22T00:20:20Z

@pytorchbot merge

pytorchmergebot · 2022-12-22T00:23:44Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2022-12-22T00:43:57Z

Merge failed

Reason: 2 additional jobs have failed, first few of them are: trunk ,trunk / linux-focal-rocm5.3-py3.8 / test (default, 2, 2, linux.rocm.gpu)

Details for Dev Infra team

Raised by workflow job

vors · 2022-12-22T05:11:53Z

Seems unrelated...

vors · 2022-12-22T05:11:59Z

@pytorchbot merge

vors · 2022-12-22T05:13:07Z

@pytorchbot sudo merge

pytorch-bot · 2022-12-22T05:13:09Z

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: argument command: invalid choice: 'sudo' (choose from 'merge', 'revert', 'rebase', 'label', 'drci')

usage: @pytorchbot [-h] {merge,revert,rebase,label,drci} ...

Try @pytorchbot --help for more info.

kit1980 · 2022-12-22T05:13:09Z

@pytorchbot merge -f "Pre-existing ROCm issues"

pytorchmergebot · 2022-12-22T05:14:51Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

vors · 2022-12-22T05:17:34Z

@kit1980 🙇

This is a follow-up from #89660 There is another place that needs to be updated. I think this time I covered all of them... Pull Request resolved: #91424 Approved by: https://github.com/malfet

dagitses · 2023-01-04T18:50:11Z

What are the edge cases under which // resolves to the top-level workspace?

vors · 2023-01-04T19:23:21Z

There are 3 flavors:

//...
@//...
@pytorch//...

From our expirience in Cruise, only the last one provides an unambiguous stable resolution.
The //... one always resolves to the top namespace which is not what we want.

github-actions bot added the module: mkldnn Related to Intel IDEEP or oneDNN (a.k.a. mkldnn) integration label Nov 24, 2022

pytorchbot added the open source label Nov 24, 2022

albanD requested a review from kit1980 November 29, 2022 14:23

albanD added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Nov 29, 2022

kit1980 self-assigned this Nov 29, 2022

pytorchmergebot force-pushed the sergei/master/add-pytorch branch from 30d64da to d1befe9 Compare December 20, 2022 21:17

This comment was marked as resolved.

Sign in to view

vors added 2 commits December 20, 2022 21:37

Use @pytorch in the third_party folder

eed1bc4

Use @pytorch in the BUILD.bazel

cc7bea3

vors force-pushed the sergei/master/add-pytorch branch from d1befe9 to cc7bea3 Compare December 20, 2022 21:37

kit1980 approved these changes Dec 22, 2022

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Dec 22, 2022

pytorchmergebot added the Merged label Dec 22, 2022

pytorchmergebot closed this in 39306c1 Dec 22, 2022

vors mentioned this pull request Dec 27, 2022

[bazel] Add @pytorch in tools/bazel.bzl #91424

Closed

Use @pytorch// in bazel build files #89660

Use @pytorch// in bazel build files #89660

Conversation

vors commented Nov 24, 2022 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/89660

❌ 2 Failures

Uh oh!

linux-foundation-easycla bot commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vors commented Nov 24, 2022

Uh oh!

kit1980 commented Nov 27, 2022

Uh oh!

kit1980 commented Nov 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vors commented Nov 27, 2022

Uh oh!

kit1980 commented Nov 28, 2022

Uh oh!

vors commented Nov 29, 2022

Uh oh!

vors commented Dec 13, 2022

Uh oh!

kit1980 commented Dec 15, 2022

Uh oh!

vors commented Dec 20, 2022

Uh oh!

pytorch-bot bot commented Dec 20, 2022

Uh oh!

vors commented Dec 20, 2022

Uh oh!

pytorch-bot bot commented Dec 20, 2022

Uh oh!

sanchitintel commented Dec 20, 2022

Uh oh!

pytorchmergebot commented Dec 20, 2022

Uh oh!

pytorchmergebot commented Dec 20, 2022

Uh oh!

This comment was marked as resolved.

kit1980 commented Dec 20, 2022

Uh oh!

vors commented Dec 20, 2022

Uh oh!

facebook-github-bot commented Dec 20, 2022

Uh oh!

vors commented Dec 21, 2022

Uh oh!

kit1980 commented Dec 22, 2022

Uh oh!

pytorchmergebot commented Dec 22, 2022

Merge started

Uh oh!

pytorchmergebot commented Dec 22, 2022

Merge failed

Uh oh!

vors commented Dec 22, 2022

Uh oh!

vors commented Dec 22, 2022

Uh oh!

vors commented Dec 22, 2022

Uh oh!

pytorch-bot bot commented Dec 22, 2022

Uh oh!

kit1980 commented Dec 22, 2022

Uh oh!

pytorchmergebot commented Dec 22, 2022

Merge started

Uh oh!

vors commented Dec 22, 2022

Uh oh!

dagitses commented Jan 4, 2023

Uh oh!

Use `@pytorch//` in bazel build files #89660

Use `@pytorch//` in bazel build files #89660

vors commented Nov 24, 2022 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Nov 24, 2022 •

edited

Loading

linux-foundation-easycla bot commented Nov 24, 2022 •

edited

Loading

kit1980 commented Nov 27, 2022 •

edited

Loading