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

Bazel 0.25 breaks upb build: Output artifact '_objs/upb_lib/descriptor.upb.pic.o' not under package directory 'external/com_google_protobuf' for target '@com_google_protobuf//:descriptor_proto' #8254

Closed
haberman opened this issue May 8, 2019 · 10 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: bug

Comments

@haberman
Copy link
Contributor

haberman commented May 8, 2019

Description of the problem / feature request:

Bazel 0.25 breaks the build of https://github.com/protocolbuffers/upb.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

$ wget https://github.com/bazelbuild/bazel/releases/download/0.25.1/bazel-0.25.1-linux-x86_64
$ chmod a+x bazel-0.25.1-linux-x86_64
$ git clone --single-branch --branch bazel25 https://github.com/haberman/upb.git
$ cd upb
$ ../bazel-0.25.1-linux-x86_64 build :descriptor_upbproto

I get the following output/error:

ERROR: /usr/local/google/home/haberman/.cache/bazel/_bazel_haberman/3d6dddcf72cff0e2141218f5c0401c8f/external/com_google_protobuf/BUILD:284:2: in //:build_defs.bzl%_upb_proto_library_aspect aspect on proto_library rule @com_google_protobuf//:descriptor_proto:
Traceback (most recent call last):
        File "/usr/local/google/home/haberman/.cache/bazel/_bazel_haberman/3d6dddcf72cff0e2141218f5c0401c8f/external/com_google_protobuf/BUILD", line 284                          
                //:build_defs.bzl%_upb_proto_library_aspect(...)
        File "/tmp/foo/upb/build_defs.bzl", line 357, in _upb_proto_aspect_impl
                cc_library_func(ctx = ctx, hdrs = files.hdrs, srcs =..., ...]))
        File "/tmp/foo/upb/build_defs.bzl", line 288, in cc_library_func
                cc_common.compile(actions = ctx.actions, feature_con..., <5 more arguments>)                                                                                       
Output artifact '_objs/upb_lib/descriptor.upb.pic.o' not under package directory 'external/com_google_protobuf' for target '@com_google_protobuf//:descriptor_proto'               
ERROR: Analysis of target '//:descriptor_upbproto' failed; build aborted: Analysis of target '@com_google_protobuf//:descriptor_proto' failed; build aborted    

What operating system are you running Bazel on?

Linux.

What's the output of bazel info release?

release 0.25.1

Any other information, logs, or outputs that you want to share?

This seems related to my StackOverflow question where I am wondering how to properly write rules/aspects that operate on external repositories: https://stackoverflow.com/questions/55796252/how-to-write-bazel-rules-that-work-with-external-repositories

I am very receptive to hearing about how to write my rule/aspect to make it simpler and avoid this error.

@haberman
Copy link
Contributor Author

haberman commented May 9, 2019

Any idea @hlopko @oquenchil ?

@hlopko hlopko added P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: bug labels May 9, 2019
@oquenchil
Copy link
Contributor

The problem is that here we are calling getPackageName() instead of getPackageIdentifier(), so the repo section of the package got dropped. This problems exists at head. I will send a fix. I can't think of a workaround at the moment.

0.26 is a week away, would it be possible for you to wait until then?

@aehlig
Copy link
Contributor

aehlig commented May 9, 2019

[...] This problems exists at head. I will send a fix. I can't think of a workaround at the moment.

0.26 is a week away, would it be possible for you to wait until then?

Note that the baseline for 0.26 (#7499) is already determined. Are your requesting a cherry-pick? That would require a rgression with respect to 0.25. Is a patch-release needed for 0.25 (#7498)?

@oquenchil
Copy link
Contributor

I have sent the fix for review. It would need to be cherry-picked into 0.26. I don't know if 0.25 needs to be patched. I would imagine that this depends on how many people this affects. Joshua if I remember correctly from our chat conversations you were still playing around experimenting with this API. Did this make it into any of your releases and does it affect a lot of users?

bazel-io pushed a commit that referenced this issue May 9, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
aehlig pushed a commit that referenced this issue May 9, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
@haberman
Copy link
Contributor Author

haberman commented May 9, 2019

Did this make it into any of your releases and does it affect a lot of users?

gRPC is an important customer of mine who is more or less blocked until I can get this working. That's why I have been spending so much effort on it. I found a way to get it working finally in 0.24.1 but then 0.25 regressed me.

If 0.26 is a week away, I am ok waiting for that. But I don't want to wait much longer than that.

aehlig pushed a commit that referenced this issue May 10, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
@oquenchil
Copy link
Contributor

Thanks Joshua. The fix 942f7cf has been cherrypicked already.

aehlig pushed a commit that referenced this issue May 10, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
@dkelmer
Copy link
Contributor

dkelmer commented May 10, 2019

@haberman I'm also creating a patch release with the fix for this so you'll be able to use 0.25.2 later today

@haberman
Copy link
Contributor Author

Thanks all for the quick resolution!

bazel-io pushed a commit that referenced this issue May 10, 2019
Baseline: 0366246

Cherry picks:

   + 3f7f255:
     Windows: fix native test wrapper's arg. escaping
   + afeb8d0:
     Flip --incompatible_windows_escape_jvm_flags
   + 4299b65:
     Sort DirectoryNode children to ensure validity.
   + 231270c:
     Conditionally use deprecated signature for initWithContentsOfURL
   + 75a3a53:
     Add http_archive entries for testing with various JDK versions.
   + 4a6354a:
     Now that ubuntu1804 uses JDK 11, remove explicit
     ubuntu1804_java11 tests.
   + ae102fb:
     Fix wrong name of ubuntu1804_javabase9 task.
   + 0020a97:
     Remove @executable_path/Frameworks from rpaths
   + 130f86d:
     Download stderr/stdout to a temporary FileOutErr
   + 2ab3866:
     Release 0.25.0 (2019-05-01)
   + ed48a4a5fddbd93b057c3aa726e15720d79dcf8f:
     Add implementation to removed methods to address
     #8226
   + 81aefe7:
     Remove unsupported cpu attribute from cc_toolchains.
   + cccced1:
     Release 0.25.1 (2019-05-07)
   + 0900660d67b53a56a13d1fa16a788e4cecbb1c0e:
     Use package identifier instead of package name
   + 85a5a2b:
     Configure @androidsdk//:emulator_x86 and :emulator_arm to point
     to the unified emulator binary

* Add fix for #8254
* Add fix for #8280
aehlig pushed a commit that referenced this issue May 13, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
@oquenchil
Copy link
Contributor

Is everything working correctly Joshua?

bung-wg2 pushed a commit to bung-wg2/bazel that referenced this issue May 15, 2019
Baseline: 0366246

Cherry picks:

   + 3f7f255:
     Windows: fix native test wrapper's arg. escaping
   + afeb8d0:
     Flip --incompatible_windows_escape_jvm_flags
   + 4299b65:
     Sort DirectoryNode children to ensure validity.
   + 231270c:
     Conditionally use deprecated signature for initWithContentsOfURL
   + 75a3a53:
     Add http_archive entries for testing with various JDK versions.
   + 4a6354a:
     Now that ubuntu1804 uses JDK 11, remove explicit
     ubuntu1804_java11 tests.
   + ae102fb:
     Fix wrong name of ubuntu1804_javabase9 task.
   + 0020a97:
     Remove @executable_path/Frameworks from rpaths
   + 130f86d:
     Download stderr/stdout to a temporary FileOutErr
   + 2ab3866:
     Release 0.25.0 (2019-05-01)
   + ed48a4a5fddbd93b057c3aa726e15720d79dcf8f:
     Add implementation to removed methods to address
     bazelbuild#8226
   + 81aefe7:
     Remove unsupported cpu attribute from cc_toolchains.
   + cccced1:
     Release 0.25.1 (2019-05-07)
   + 0900660d67b53a56a13d1fa16a788e4cecbb1c0e:
     Use package identifier instead of package name
   + 85a5a2b:
     Configure @androidsdk//:emulator_x86 and :emulator_arm to point
     to the unified emulator binary

* Add fix for bazelbuild#8254
* Add fix for bazelbuild#8280
@haberman
Copy link
Contributor Author

@oquenchil yes I have everything working now, thanks! :)

aehlig pushed a commit that referenced this issue May 17, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
aehlig pushed a commit that referenced this issue May 17, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
aehlig pushed a commit that referenced this issue May 20, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
aehlig pushed a commit that referenced this issue May 21, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
aehlig pushed a commit that referenced this issue May 22, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
aehlig pushed a commit that referenced this issue May 23, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
aehlig pushed a commit that referenced this issue May 23, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
aehlig pushed a commit that referenced this issue May 24, 2019
Reported in: #8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
Reported in: bazelbuild#8254

Added test testApiWithAspectsOnTargetsInExternalRepos()

RELNOTES:none
PiperOrigin-RevId: 247412486
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
Baseline: 0366246

Cherry picks:

   + 3f7f255:
     Windows: fix native test wrapper's arg. escaping
   + afeb8d0:
     Flip --incompatible_windows_escape_jvm_flags
   + 4299b65:
     Sort DirectoryNode children to ensure validity.
   + 231270c:
     Conditionally use deprecated signature for initWithContentsOfURL
   + 75a3a53:
     Add http_archive entries for testing with various JDK versions.
   + 4a6354a:
     Now that ubuntu1804 uses JDK 11, remove explicit
     ubuntu1804_java11 tests.
   + ae102fb:
     Fix wrong name of ubuntu1804_javabase9 task.
   + 0020a97:
     Remove @executable_path/Frameworks from rpaths
   + 130f86d:
     Download stderr/stdout to a temporary FileOutErr
   + 2ab3866:
     Release 0.25.0 (2019-05-01)
   + ed48a4a5fddbd93b057c3aa726e15720d79dcf8f:
     Add implementation to removed methods to address
     bazelbuild#8226
   + 81aefe7:
     Remove unsupported cpu attribute from cc_toolchains.
   + cccced1:
     Release 0.25.1 (2019-05-07)
   + 0900660d67b53a56a13d1fa16a788e4cecbb1c0e:
     Use package identifier instead of package name
   + 85a5a2b:
     Configure @androidsdk//:emulator_x86 and :emulator_arm to point
     to the unified emulator binary

* Add fix for bazelbuild#8254
* Add fix for bazelbuild#8280
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
    Reported in: bazelbuild/bazel#8254

    Added test testApiWithAspectsOnTargetsInExternalRepos()

    RELNOTES:none
    PiperOrigin-RevId: 247412486
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

No branches or pull requests

5 participants