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

Add support for Bazel 6 & 7 #2009

Merged
merged 3 commits into from
Mar 5, 2024
Merged

Conversation

fzakaria
Copy link
Contributor

Previous PR #2006 added support for Bazel 7 but some of the changes in Bazel 7 are not backwards compatible with 6.

This is a problem for some of our consumers that embed our project in other Bazel workspaces.

Specifically, the name of the runfiles directory was changed from the workspace name to be '_main'.

The solution adopted here is to use @python_rules and programmatically determine the runfiles directory relative to the lit.cfg.py file.

@fzakaria
Copy link
Contributor Author

@sjain-stanford do you want try this?
I'm still not sure what's the more idiomatic way -- this approach or doing it in the BUILD file itself.

@sjain-stanford
Copy link
Contributor

@fzakaria thanks for working on this. I gave it a shot but since we're on slightly older LLVM, couldn't verify the lit tests work (hit build errors). If you've tested it on your end, I say we either land and circle back if I hit issues, or keep this open and I reach out when we do our next integrate (in ~2 weeks). WDYT?

@fzakaria
Copy link
Contributor Author

Let's revisit when you integrate to confirm since the purpose of the change is to support your integrate.
I don't expect the PR to go stale too much.

@sjain-stanford
Copy link
Contributor

Okay I'm back to this as our integrate expectedly fails:

Executing tests from @stablehlo//stablehlo/testdata:scatter_no_xla_unique_indices_shape_uint16_1_1__scatterindices__0__updateshape__1___updatewindowdims-2800941818176172076.mlir.test                     
-----------------------------------------------------------------------------                                                                                                                              
lit.py: /home/sambhav.jain/.cache/bazel/_bazel_sambhav.jain/3cd6ec0a5b4a7761b28ebc109f1a1f60/external/llvm-raw/llvm/utils/lit/lit/TestingConfig.py:152: fatal: unable to parse config file '/home/sambhav.j
ain/.cache/bazel/_bazel_sambhav.jain/3cd6ec0a5b4a7761b28ebc109f1a1f60/execroot/mlir-tcp/bazel-out/k8-fastbuild/bin/external/stablehlo/stablehlo/testdata/scatter_no_xla_unique_indices_shape_uint16_1_1__sc
atterindices__0__updateshape__1___updatewindowdims-2800941818176172076.mlir.test.runfiles/mlir-tcp/external/stablehlo/stablehlo/testdata/lit.site.cfg.py', traceback: Traceback (most recent call last):   
  File "/home/sambhav.jain/.cache/bazel/_bazel_sambhav.jain/3cd6ec0a5b4a7761b28ebc109f1a1f60/external/llvm-raw/llvm/utils/lit/lit/TestingConfig.py", line 140, in load_from_path                           
    exec(compile(data, path, "exec"), cfg_globals, None)                                                                                                                                                   
  File "/home/sambhav.jain/.cache/bazel/_bazel_sambhav.jain/3cd6ec0a5b4a7761b28ebc109f1a1f60/execroot/mlir-tcp/bazel-out/k8-fastbuild/bin/external/stablehlo/stablehlo/testdata/scatter_no_xla_unique_indic
es_shape_uint16_1_1__scatterindices__0__updateshape__1___updatewindowdims-2800941818176172076.mlir.test.runfiles/mlir-tcp/external/stablehlo/stablehlo/testdata/lit.site.cfg.py", line 21, in <module>     
    lit_config.load_config(config, os.path.join(os.environ['TEST_SRCDIR'], '_main') + "/stablehlo/testdata/lit.cfg.py")                                                                                    
  File "/home/sambhav.jain/.cache/bazel/_bazel_sambhav.jain/3cd6ec0a5b4a7761b28ebc109f1a1f60/external/llvm-raw/llvm/utils/lit/lit/LitConfig.py", line 154, in load_config                                  
    config.load_from_path(path, self)                                                                                                                                                                      
  File "/home/sambhav.jain/.cache/bazel/_bazel_sambhav.jain/3cd6ec0a5b4a7761b28ebc109f1a1f60/external/llvm-raw/llvm/utils/lit/lit/TestingConfig.py", line 127, in load_from_path                           
    f = open(path)                                                                                                                                                                                         
FileNotFoundError: [Errno 2] No such file or directory: '/home/sambhav.jain/.cache/bazel/_bazel_sambhav.jain/3cd6ec0a5b4a7761b28ebc109f1a1f60/execroot/mlir-tcp/bazel-out/k8-fastbuild/bin/external/stableh
lo/stablehlo/testdata/scatter_no_xla_unique_indices_shape_uint16_1_1__scatterindices__0__updateshape__1___updatewindowdims-2800941818176172076.mlir.test.runfiles/_main/stablehlo/testdata/lit.cfg.py'     

Will re-try after cherry-picking this change.

@sjain-stanford
Copy link
Contributor

@fzakaria do you mind rebasing this branch on latest HEAD? I'm seeing some merge conflicts that I'm not too familiar resolving:

Auto-merging MODULE.bazel.lock                                                                                                                                                                             
CONFLICT (content): Merge conflict in MODULE.bazel.lock                                                                                                                                                    
Auto-merging stablehlo/conversions/linalg/tests/BUILD.bazel                                                                                                                                                
CONFLICT (content): Merge conflict in stablehlo/conversions/linalg/tests/BUILD.bazel                                                                                                                       
Auto-merging stablehlo/conversions/tosa/tests/BUILD.bazel                                                                                                                                                  
CONFLICT (content): Merge conflict in stablehlo/conversions/tosa/tests/BUILD.bazel                                                                                                                         
Auto-merging stablehlo/testdata/BUILD.bazel                                                                                                                                                                
CONFLICT (content): Merge conflict in stablehlo/testdata/BUILD.bazel                                                                                                                                       
Auto-merging stablehlo/tests/BUILD.bazel                                                                                                                                                                   
CONFLICT (content): Merge conflict in stablehlo/tests/BUILD.bazel                                                                                                                                          
Automatic merge failed; fix conflicts and then commit the result.

Previous PR openxla#2006 added support for Bazel 7 but some of the changes in
Bazel 7 are not backwards compatible with 6.

This is a problem for some of our consumers that embed our project in
other Bazel workspaces.

Specifically, the name of the runfiles directory was changed from the
workspace name to be '_main'.

The solution adopted here is to use @python_rules and programmatically
determine the runfiles directory relative to the lit.cfg.py file.
@fzakaria
Copy link
Contributor Author

fzakaria commented Mar 5, 2024

@sjain-stanford updated.

@sjain-stanford
Copy link
Contributor

@fzakaria confirming the patch works for downstream stablehlo integrations 🎉

$ bazel test @stablehlo//...

INFO: Analyzed 4183 targets (25 packages loaded, 9891 targets configured).                                                                                                                                                                                                     
INFO: Found 152 targets and 4031 test targets...                                                                                                                                                                                                                               
[10,255 / 10,277] 78 / 4031 tests; checking cached actions; last test: @stablehlo//stablehlo/t...
.
.
.                                                                                                                                                                        
@stablehlo//stablehlo/tests:vhlo/vhlo_to_version_downgrade_invalid.0_9_0.mlir.test PASSED in 1.1s                                                                                                                                                                              
@stablehlo//stablehlo/tests:vhlo/vhlo_to_version_invalid.mlir.test       PASSED in 1.5s                                                                                                                                                                                        
                                                                                                                                                                                                                                                                               
Executed 4031 out of 4031 tests: 4031 tests pass.                     

I'm still not sure what's the more idiomatic way -- this approach or doing it in the BUILD file itself.

Since the llvm lit config setup is pretty much munging paths anyways, I don't have a strong preference one way or another (doing it in lit.cfg.py vs BUILD). Please go with your best judgment here.

Once this lands, I will go ahead and bump to stablehlo@HEAD in mlir-tcp (cruise-automation/mlir-tcp#48).

Copy link
Contributor

@sjain-stanford sjain-stanford left a comment

Choose a reason for hiding this comment

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

Thanks for helping restore the builds in our downstream repos.

@fzakaria fzakaria merged commit d214e2e into openxla:main Mar 5, 2024
10 checks passed
@fzakaria fzakaria deleted the runfiles-follow-up branch March 5, 2024 23:21
sjain-stanford added a commit to sjain-stanford/mlir-tcp that referenced this pull request Mar 5, 2024
sjain-stanford added a commit to cruise-automation/mlir-tcp that referenced this pull request Mar 6, 2024
[LLVM](https://github.com/llvm/llvm-project/tree/e5ed7b6e2fd368b722b6359556cd0125881e7638)
(Feb 27)

[StableHlo](https://github.com/openxla/stablehlo/tree/d214e2e2bd568a80a9f25515907b64dd7b98c538)
(March 5)**

[Torch-MLIR](https://github.com/llvm/torch-mlir/tree/a86e89ecb5c7929a39a38743fb7cacadf1ff41bb)
(Mar 4)


DONE:
- [x] **This is the stablehlo commit right before Bazel 7 (bzlmod)
changes landed (openxla/stablehlo#2006) which
breaks downstream integrations. Stablehlo devs have kindly provided a
fix at openxla/stablehlo#2009 - if it works,
I'll revise the stablehlo commit to HEAD.
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.

3 participants