search

bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.18k stars 4.06k forks source link

incompatible_disable_deprecated_attr_params: Cleanup skylark 'attr' API #5818

Closed c-parsons closed 5 years ago

c-parsons commented 6 years ago

This is a tracking issue for offering a migration solution for --incompatible_disable_deprecated_attr_params

This flag would turn down a number of deprecated APIs pertaining to attr. For example, the single_file parameter would no longer be used.

laurentlb commented 5 years ago

Migration notes

When creating attributes, for example attr.label_list, use allow_empty instead of non_empty. This can be fixed automatically by running Buildifier on the bzl files, with --lint=fix.

laurentlb commented 5 years ago

Main blocker for this is that "protobuf 3.6.1" doesn't have the fix, it still uses single_file = True, although it was fixed at head in August (https://github.com/protocolbuffers/protobuf/commit/d5f0dac497f833d06f92d246431f4f2f42509e04).

I'm removing the migration-ready label, because many repositories depend on protobuf.

jin commented 5 years ago

The latest Protobuf tag (3.6.1.3) still uses single_file = True.

@acozzette could you create a new tag (3.6.1.4?) with https://github.com/protocolbuffers/protobuf/commit/d5f0dac497f833d06f92d246431f4f2f42509e04 please?

philwo commented 5 years ago

This flag was not flipped in time for the Bazel 0.23.0 release and will thus be postponed to Bazel 0.24.0.

katre commented 5 years ago

This issue was tagged as "breaking-change-0.24" but does not appear ready to be flipped in the 0.24.0 release. If this is incorrect please comment on that issue and discuss with me.

c-parsons commented 5 years ago

This flag is ready to be flipped, I believe, but we did not actually submit a change to flip it. My bad. If this has missed the cut for the 0.24.0 release, then it'll have to wait for the next one. (It's not urgent)

I'll propose a commit to flip this flag.

laurentlb commented 5 years ago

The flag was intentionally not flipped because it would break a huge number of repositories. Now that Protobuf 3.7.0 has been released, we may be able to update them.

jin commented 5 years ago

3.7.0 is not buildable with Bazel yet. See https://github.com/protocolbuffers/protobuf/pull/5811

davido commented 5 years ago

Now that Protobuf 3.7.0 has been released, we may be able to update them.

+1. I filed this issue to track protobuf upgrade in Bazel. I also started to work on it and realized the problem. However, given that protocolbuffers/protobuf#5811 is a trivial change and given that actually protobuf fork is used in Bazel tree, we could proceed with the upgrade even before this PR was merged.

I could look into upgrading protobuf in Bazel tree.

laurentlb commented 5 years ago

Many downstream projects are still broken (https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/67). We should update them to use the latest protobuf.

davido commented 5 years ago

We should update them to use the latest protobuf.

Right, including upgrading Bazel itself (https://github.com/bazelbuild/bazel/pull/7681 and https://github.com/bazelbuild/bazel/pull/7775).

laurentlb commented 5 years ago

Status update. Flag flip still blocked by:

davido commented 5 years ago

@laurentb What about protobuf upgrade in Bazel itself: pending (#7681 and #7775)? Shouldn't you mention this here as well?

laurentlb commented 5 years ago

protobuf.bzl was updated separately (https://github.com/bazelbuild/bazel/commit/ad503849e78b98d762f03168de5a336904280150).

To identify breakages, I checked https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/114

davido commented 5 years ago

protobuf.bzl was updated separately (ad50384).

Ah, right. I forgot that Bazel is using forked version of Protobuf.

brandjon commented 5 years ago

Looks like this breaks rules_python but wasn't caught by tests. Filed bazelbuild/rules_python#191.

brandjon commented 5 years ago

Disregard the above, user error on my part.