Bazel downstream failure: Tensorflow visibility issue

[SalmaSamy]

Tensorflow is failing in Bazel downstream with this error:

After checking the runtime repository, I found that :support target is visible to a package group which ONLY includes targets in the same project, while it is used as a dependency in Tensorflow repository:

Please update this target to be visible from needed projects

[SalmaSamy]

/cc @ezhulenev

[meteorcloudy]

Link to broken build: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2776#0184f510-f91a-4155-8d7d-3ce630837559

[meteorcloudy]

Related change in Bazel@HEAD: bazelbuild/bazel@e899d85

Incompatible changes: bazelbuild/bazel#16391, bazelbuild/bazel#16355, bazelbuild/bazel#16323

/cc @brandjon

[meteorcloudy]

@brandjon Maybe we should cherry pick bazelbuild/bazel@1473988 back to 5.x to make it easier for Bazel projects to adapt the new syntax?

[mohantym]

Hi @SalmaSamy !
Thanks for sharing those observation on Bazel downstream failure.
Would it be possible to share the commands to investigate this issue on our side.

Thank you!

[meteorcloudy]

Yes, you can reproduce this issue with

export USE_BAZEL_VERSION=last_green
bazelisk build //tensorflow/tools/pip_package:build_pip_package

[brandjon]

This appears to be caused by the same package_group as issue #58168, the @tf_runtime//:friends target. As in that issue, the possible workarounds include:

1. Replace //... with public, which only works in Bazel >= 6.0
2. Update the visibility of the affected targets to be //visibility:public in place of the friends package group
3. Opt out of the incompatible change in Bazel 6.0 by setting --incompatible_fix_package_group_reporoot_syntax=false

I suggest that (2) is the simplest option, inconvenient though it may be. Of course, it can be factored back into the friends package group once compatibility with < 6.0 is no longer needed.

As for Yun's suggestion above to backport public to 5.x, I think that should be possible. I wonder how costly the above options are for users though.

[sachinprasadhs]

@brandjon , Thanks for the detailed explanation, do you think there is any action item from Tensorflow repo.

[brandjon]

@sachinprasadhs, I'd like to know if it's a burden for you to migrate away from @tf_runtime//:friends and replace it with visibility = ["//visibility:public"], or if that's a trivial change for you. Same for any other package groups using //....

I'd also like to know if you need long term support for 5.x or if it's just needed for migration.

[sachinprasadhs]

@learning-to-play , @nitins17 , Could you please comment on the above request. Thanks!

[brandjon]

It looks like 5.4 will go ahead without adding support for public. As best as we can tell, it also seems like TF is the only significant case where this has impacted migration. So at the moment we're not planning on backporting this, given that there is the workaround of using //visibility:public in place of a package group.

[SalmaSamy]

Hi @sachinprasadhs, any updates on using one of the solution for this?

[anishlukk123]

any updates on this?

[meteorcloudy]

I believe this is already fixed at TF HEAD

[google-ml-butler]

Are you satisfied with the resolution of your issue?
Yes
No