Skip to content
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

fix(bzlmod): allow both root module and our module to call cuda.local_toolchain #264

Merged
merged 3 commits into from
Aug 12, 2024

Conversation

cloudhan
Copy link
Collaborator

@cloudhan cloudhan commented Aug 7, 2024

Superserde #239

In #263 (comment) a new error popup as expected.

It is caused by the unittest secretly setup our module (rules_cuda, current root module as of invocation of bazel command) as an external module under testing, then the local_cuda is ignore in this case. We shall allow call to module_extenion in both root module and our module.

This in principle also fixes #238.

@cloudhan cloudhan force-pushed the cloudhan/our-module-local-cuda branch from b5f4392 to 8246bbc Compare August 7, 2024 15:42
@cloudhan cloudhan marked this pull request as ready for review August 7, 2024 15:46
@cloudhan cloudhan force-pushed the cloudhan/canonicalize-label branch from b82e0d9 to fb92d37 Compare August 7, 2024 16:15
Base automatically changed from cloudhan/canonicalize-label to main August 7, 2024 16:41
@cloudhan cloudhan force-pushed the cloudhan/our-module-local-cuda branch from 8246bbc to 73f0723 Compare August 7, 2024 16:42
@@ -8,6 +8,10 @@ bazel_dep(name = "bazel_skylib", version = "1.4.2")
bazel_dep(name = "platforms", version = "0.0.6")

cuda = use_extension("@rules_cuda//cuda:extensions.bzl", "toolchain")
cuda.local_toolchain(
name = "local_cuda",
toolkit_path = "",
Copy link
Member

Choose a reason for hiding this comment

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

If a user doesn't have an install at the default path does this cause a failure if they also override it in their own module file? Should we be loading the extension with dev_dependency = True to avoid this if so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, this does not trigger analysis error anymore thanks to #209. The generated local_cuda repo is as follows

root@c578fa5b9b4e ~/rules_cuda (cloudhan/our-module-local-cuda)# tree bazel-rules_cuda/external/_main\~toolchain\~local_cuda/
bazel-rules_cuda/external/_main~toolchain~local_cuda/
|-- BUILD -> /root/rules_cuda/cuda/runtime/BUILD.local_cuda_disabled
|-- REPO.bazel
|-- WORKSPACE
|-- defs.bzl
`-- toolchain
    |-- BUILD
    |-- clang
    |   `-- BUILD
    `-- disabled
        `-- BUILD -> /root/rules_cuda/cuda/templates/BUILD.local_toolchain_disabled

3 directories, 7 files
root@c578fa5b9b4e ~/rules_cuda (cloudhan/our-module-local-cuda)# cat bazel-rules_cuda/external/_main\~toolchain\~local_cuda/BUILD
load("@bazel_skylib//rules:common_settings.bzl", "bool_setting")

package(
    default_visibility = ["//visibility:public"],
)

bool_setting(
    name = "valid_toolchain_found",
    build_setting_default = False,
)

@cloudhan cloudhan requested a review from jsharpe August 9, 2024 01:03
@cloudhan cloudhan force-pushed the cloudhan/our-module-local-cuda branch 2 times, most recently from e20a1b6 to 01e0709 Compare August 12, 2024 14:43
@cloudhan cloudhan force-pushed the cloudhan/our-module-local-cuda branch from 01e0709 to 7688bae Compare August 12, 2024 15:06
@cloudhan cloudhan merged commit f00f640 into main Aug 12, 2024
16 checks passed
@cloudhan cloudhan deleted the cloudhan/our-module-local-cuda branch August 12, 2024 16:13
anakinxc referenced this pull request in secretflow/spu Aug 14, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [rules_cuda](https://togithub.com/bazel-contrib/rules_cuda) |
http_archive | patch | `v0.2.1` -> `v0.2.2` |

---

### Release Notes

<details>
<summary>bazel-contrib/rules_cuda (rules_cuda)</summary>

###
[`v0.2.2`](https://togithub.com/bazel-contrib/rules_cuda/releases/tag/v0.2.2)

[Compare
Source](https://togithub.com/bazel-contrib/rules_cuda/compare/v0.2.1...v0.2.2)

#### `WORKSPACE` code

```starlark
load("@&#8203;bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
    name = "rules_cuda",
    sha256 = "b066750579f33e93e9dc55b8ee2067b525d863c1ddcf09b47a6332c39f0701fb",
    strip_prefix = "rules_cuda-v0.2.2",
    urls = ["https://github.com/bazel-contrib/rules_cuda/releases/download/v0.2.2/rules_cuda-v0.2.2.tar.gz"],
)

load("@&#8203;rules_cuda//cuda:repositories.bzl", "register_detected_cuda_toolchains", "rules_cuda_dependencies")
rules_cuda_dependencies()
register_detected_cuda_toolchains()
```

#### What's Changed

- Filter attrs properly for cuda_test by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/180](https://togithub.com/bazel-contrib/rules_cuda/pull/180)
- Fix cuda_test by [@&#8203;hofbi](https://togithub.com/hofbi) in
[https://github.com/bazel-contrib/rules_cuda/pull/181](https://togithub.com/bazel-contrib/rules_cuda/pull/181)
- Add v0.2.1 to docs by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/184](https://togithub.com/bazel-contrib/rules_cuda/pull/184)
- Pass through `--sysroot` to host compiler by
[@&#8203;lalten](https://togithub.com/lalten) in
[https://github.com/bazel-contrib/rules_cuda/pull/185](https://togithub.com/bazel-contrib/rules_cuda/pull/185)
- Add cuda_binary macro by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/186](https://togithub.com/bazel-contrib/rules_cuda/pull/186)
- Move non glob files out of glob by
[@&#8203;hofbi](https://togithub.com/hofbi) in
[https://github.com/bazel-contrib/rules_cuda/pull/192](https://togithub.com/bazel-contrib/rules_cuda/pull/192)
- Disallow autoupdate nccl by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/193](https://togithub.com/bazel-contrib/rules_cuda/pull/193)
- eliminate cpu architecture constraint for clang by
[@&#8203;dmellosanjay](https://togithub.com/dmellosanjay) in
[https://github.com/bazel-contrib/rules_cuda/pull/208](https://togithub.com/bazel-contrib/rules_cuda/pull/208)
- Check nvcc version before adding `--dopt on` flags by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/212](https://togithub.com/bazel-contrib/rules_cuda/pull/212)
- Add alwayslink to cuda_binary and cuda_test macros by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/210](https://togithub.com/bazel-contrib/rules_cuda/pull/210)
- Add additional tests for LTS releases by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/215](https://togithub.com/bazel-contrib/rules_cuda/pull/215)
- Fix cquery-ing with cuda targets by
[@&#8203;mvukov](https://togithub.com/mvukov) in
[https://github.com/bazel-contrib/rules_cuda/pull/209](https://togithub.com/bazel-contrib/rules_cuda/pull/209)
- Propose new solution for know issue (nvcc filesystem race condition)
by [@&#8203;hofbi](https://togithub.com/hofbi) in
[https://github.com/bazel-contrib/rules_cuda/pull/216](https://togithub.com/bazel-contrib/rules_cuda/pull/216)
- Fix a typo in `if_cuda` doc by
[@&#8203;rygx](https://togithub.com/rygx) in
[https://github.com/bazel-contrib/rules_cuda/pull/222](https://togithub.com/bazel-contrib/rules_cuda/pull/222)
- Ignore MODULE.bazel.lock file by
[@&#8203;rygx](https://togithub.com/rygx) in
[https://github.com/bazel-contrib/rules_cuda/pull/224](https://togithub.com/bazel-contrib/rules_cuda/pull/224)
- ci: avoid nvcc /tmp race condition by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/232](https://togithub.com/bazel-contrib/rules_cuda/pull/232)
- ci: disable doc test workflow cache to avoid excessive space wasting
by [@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/231](https://togithub.com/bazel-contrib/rules_cuda/pull/231)
- feat: Add features for compiling with -arch=all or -arch=all-major by
[@&#8203;jsharpe](https://togithub.com/jsharpe) in
[https://github.com/bazel-contrib/rules_cuda/pull/245](https://togithub.com/bazel-contrib/rules_cuda/pull/245)
- Fix spelling by [@&#8203;Vertexwahn](https://togithub.com/Vertexwahn)
in
[https://github.com/bazel-contrib/rules_cuda/pull/250](https://togithub.com/bazel-contrib/rules_cuda/pull/250)
- Change example rules_cuda version by
[@&#8203;Vertexwahn](https://togithub.com/Vertexwahn) in
[https://github.com/bazel-contrib/rules_cuda/pull/249](https://togithub.com/bazel-contrib/rules_cuda/pull/249)
- Document how to use rules_cuda with Bzlmod by
[@&#8203;Vertexwahn](https://togithub.com/Vertexwahn) in
[https://github.com/bazel-contrib/rules_cuda/pull/252](https://togithub.com/bazel-contrib/rules_cuda/pull/252)
- Do not assume libcupti.so location by
[@&#8203;tyb0807](https://togithub.com/tyb0807) in
[https://github.com/bazel-contrib/rules_cuda/pull/253](https://togithub.com/bazel-contrib/rules_cuda/pull/253)
- ci: use absolute path for XDG_CACHE_HOME as github actions and bazel
doesn't resolve `~` automatically by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/260](https://togithub.com/bazel-contrib/rules_cuda/pull/260)
- ci: cover major bazel releases in utilities tests by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/262](https://togithub.com/bazel-contrib/rules_cuda/pull/262)
- test: workaround label resolving with bzlmod by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/263](https://togithub.com/bazel-contrib/rules_cuda/pull/263)
- fix(bzlmod): allow both root module and our module to call
cuda.local_toolchain by
[@&#8203;cloudhan](https://togithub.com/cloudhan) in
[https://github.com/bazel-contrib/rules_cuda/pull/264](https://togithub.com/bazel-contrib/rules_cuda/pull/264)

#### New Contributors

- [@&#8203;dmellosanjay](https://togithub.com/dmellosanjay) made their
first contribution in
[https://github.com/bazel-contrib/rules_cuda/pull/208](https://togithub.com/bazel-contrib/rules_cuda/pull/208)
- [@&#8203;mvukov](https://togithub.com/mvukov) made their first
contribution in
[https://github.com/bazel-contrib/rules_cuda/pull/209](https://togithub.com/bazel-contrib/rules_cuda/pull/209)
- [@&#8203;rygx](https://togithub.com/rygx) made their first
contribution in
[https://github.com/bazel-contrib/rules_cuda/pull/222](https://togithub.com/bazel-contrib/rules_cuda/pull/222)
- [@&#8203;Vertexwahn](https://togithub.com/Vertexwahn) made their first
contribution in
[https://github.com/bazel-contrib/rules_cuda/pull/250](https://togithub.com/bazel-contrib/rules_cuda/pull/250)
- [@&#8203;tyb0807](https://togithub.com/tyb0807) made their first
contribution in
[https://github.com/bazel-contrib/rules_cuda/pull/253](https://togithub.com/bazel-contrib/rules_cuda/pull/253)

**Full Changelog**:
bazel-contrib/rules_cuda@v0.2.1...v0.2.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job log](https://developer.mend.io/github/secretflow/spu).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yMC4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to use cublas in a non-root bazel module?
2 participants