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

incompatible_package_name_is_a_function: Remove PACKAGE_NAME and REPOSITORY_NAME #5827

Closed
c-parsons opened this Issue Aug 8, 2018 · 16 comments

Comments

Projects
None yet
8 participants
@c-parsons
Copy link
Contributor

c-parsons commented Aug 8, 2018

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

This flag disables use of the skylark constants PACKAGE_NAME and REPOSITORY_NAME. Users should use package_name() and repository_name() instead.

Migration

  1. replace PACKAGE_NAME with package_name() call
  2. replace REPOSITORY_NAME with repository_name() call

The behavior is identical

@c-parsons c-parsons self-assigned this Aug 8, 2018

@c-parsons c-parsons changed the title Remove PACAKGE_NAME and REPOSITORY_NAME Remove PACKAGE_NAME and REPOSITORY_NAME Aug 10, 2018

@laurentlb

This comment has been minimized.

Copy link
Contributor

laurentlb commented Sep 5, 2018

Timeline:

@laurentlb laurentlb assigned laurentlb and unassigned c-parsons Sep 5, 2018

bazel-io pushed a commit that referenced this issue Sep 6, 2018

Switch the default for --incompatible_package_name_is_a_function
#5827

RELNOTES[INC]: --incompatible_package_name_is_a_function now defaults to true. The magic values PACKAGE_NAME and REPOSITORY_NAME are no longer exposed.

PiperOrigin-RevId: 211789425

bazel-io pushed a commit that referenced this issue Sep 6, 2018

Remove duplicate checks in Environment
There was some duplication between Environment and LValue.

The check in the DynamicFrame was removed. The dynamic frame should be completely separate from user values (set a value with setupDynamic, read it with dynamicLookup), they shouldn't conflict with each other.

#5827

RELNOTES: None.
PiperOrigin-RevId: 211820447
@benjaminp

This comment has been minimized.

Copy link
Contributor

benjaminp commented Sep 6, 2018

I hope major Bazel-using packages will release fixed versions before this deprecation is finished. The latest release of protobuf is still using PACKAGE_NAME, though it has been fixed on master.

@meteorcloudy

This comment has been minimized.

Copy link
Member

meteorcloudy commented Sep 10, 2018

4a8bacd is causing many projects to fail in bazel downstream job.
See https://buildkite.com/bazel/bazel-with-downstream-projects-bazel/builds/433#fc709679-63bc-4517-a4fb-02f5d8cbed2b

Most of the failing projects depends on protobuf and have the following error message:

ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/daf61e1fe31310589cae072c149a6fc3/external/com_google_protobuf/BUILD:573:1: Traceback (most recent call last):
--
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/daf61e1fe31310589cae072c149a6fc3/external/com_google_protobuf/BUILD", line 573
  | internal_gen_well_known_protos_java(srcs = WELL_KNOWN_PROTOS)
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/daf61e1fe31310589cae072c149a6fc3/external/com_google_protobuf/protobuf.bzl", line 269, in internal_gen_well_known_protos_java
  | Label(("%s//protobuf_java" % REPOSITOR...))
  | File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/daf61e1fe31310589cae072c149a6fc3/external/com_google_protobuf/protobuf.bzl", line 269, in Label
  | REPOSITORY_NAME
  | The value 'REPOSITORY_NAME' has been removed in favor of 'repository_name()', please use the latter

As @benjaminp pointed out, the release version of protobuf still uses PACKAGE_NAME, should we rollback 4a8bacd or use --incompatible_package_name_is_a_function=false for all the failing projects?

@meteorcloudy

This comment has been minimized.

Copy link
Member

meteorcloudy commented Sep 10, 2018

@davido Looks like Gerrit still use PACKAGE_NAME in its bzl file

ERROR: /Users/buildkite/builds/buildkite-imacpro-6-1/bazel-downstream-projects/gerrit/gerrit-gwtui/BUILD:23:1: Traceback (most recent call last):
--
  | File "/Users/buildkite/builds/buildkite-imacpro-6-1/bazel-downstream-projects/gerrit/gerrit-gwtui/BUILD", line 23
  | license_test(name = "ui_module_license_test", t...")
  | File "/Users/buildkite/builds/buildkite-imacpro-6-1/bazel-downstream-projects/gerrit/tools/bzl/license.bzl", line 42, in license_test
  | PACKAGE_NAME
  | The value 'PACKAGE_NAME' has been removed in favor of 'package_name()', please use the latter

Can you help fixing that?

@hlopko

This comment has been minimized.

Copy link
Contributor

hlopko commented Sep 10, 2018

cc @aehlig this is the reason why the bazel with downstream projects is in such a bad shape for release.
@laurentlb @c-parsons what is the best course of action here? Is it an option to postpone flipping the flag until 0.19? Philosophically this is a protobuf issue. Practically half of the bazel world will be broken.

@laurentlb

This comment has been minimized.

Copy link
Contributor

laurentlb commented Sep 10, 2018

Rollback commit is being submitted.

bazel-io pushed a commit that referenced this issue Sep 10, 2018

Switch back the default for --incompatible_package_name_is_a_function
The value was changed in 4a8bacd, but it is breaking downstream projects

#5827

RELNOTES: Revert the default of --incompatible_package_name_is_a_function to false.
PiperOrigin-RevId: 212297376
@davido

This comment has been minimized.

Copy link
Contributor

davido commented Sep 11, 2018

@meteorcloudy

@davido Looks like Gerrit still use PACKAGE_NAME in its bzl file
[...]
Can you help fixing that?

Done.

lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Sep 11, 2018

Bazel: Replace PACKAGE_NAME constant with package_name() function
PACKAGE_NAME was deprecated in favor of package_name() function and is
going to be removed in future Bazel releases. Moreover Bazel is trying
to set --incompatible_package_name_is_a_function=true per default to
enforce Bazel users to stop using it: [1].

[1] bazelbuild/bazel#5827

Change-Id: I4d1a6ccbeae611fdd0e9c1e9fdd89b6dd1294f36

bazel-io pushed a commit that referenced this issue Sep 11, 2018

Automated rollback of commit 4b6da43.
*** Reason for rollback ***

roll forward, after fix and new test

*** Original change description ***

Automated rollback of commit 395fbbd.

*** Reason for rollback ***

Breaking blaze guitar tests

*** Original change description ***

Allow shadowing of builtins in bzl files

This was previously allowed only with --incompatible_static_name_resolution (although it was a backward-compatible change). Removing the code path simplifies the code and makes easier to implement the static name resolution proposal.

The downside is that it will temporarily allow this corner-case:

```
x = len(.....

***

#5827

RELNOTES: None.
PiperOrigin-RevId: 212436910

bazel-io pushed a commit that referenced this issue Sep 19, 2018

Cleaner separatation between Module and Universe variables.
- In the global frame, rename "parent" into "universe" to clarify what it is.
- Function `moduleLookup` now does the right thing. It doesn't check the universe anymore.

This means that we now forbid (with --incompatible_static_name_resolution) this:
  a = len("abc")
  len = 2

#5827

RELNOTES: None.
PiperOrigin-RevId: 213647873

@dslomov dslomov changed the title Remove PACKAGE_NAME and REPOSITORY_NAME incompatible_package_name_is_a_function: Remove PACKAGE_NAME and REPOSITORY_NAME Oct 31, 2018

@dslomov

This comment has been minimized.

Copy link
Contributor

dslomov commented Oct 31, 2018

What is missing for migration: migration docs, length of migration window. After these are done, please add "migration-ready" label.

@laurentlb

This comment has been minimized.

Copy link
Contributor

laurentlb commented Nov 22, 2018

To automatically fix the code and migrate, use Buildifier:

buildifier --lint=fix --warnings=package-name,repository-name

bazel-io pushed a commit that referenced this issue Nov 22, 2018

Automated rollback of commit e4e2632.
*** Reason for rollback ***

Break half of the downstream projects in Bazel CI
https://buildkite.com/bazel/bazel-with-downstream-projects-bazel/builds/606#a41f9dca-265e-472d-91db-69117048cb62

*** Original change description ***

Flip the flag "--incompatible_package_name_is_a_function"

https://docs.bazel.build/versions/master/skylark/backward-compatibility.html#package-name-is-a-function

Fixes #5827

RELNOTES: --incompatible_package_name_is_a_function is now enabled by default
PiperOrigin-RevId: 222532723
@laurentlb

This comment has been minimized.

Copy link
Contributor

laurentlb commented Nov 26, 2018

@dslomov Why did you remove the migration-ready label?

bazel-io pushed a commit that referenced this issue Nov 29, 2018

Roll forward (3b3b469).
Many repositories have been updated.

*** Original change description ***

Automated rollback of commit e4e2632.

*** Reason for rollback ***

Break half of the downstream projects in Bazel CI
https://buildkite.com/bazel/bazel-with-downstream-projects-bazel/builds/606#a41f9dca-265e-472d-91db-69117048cb62

*** Original change description ***

Flip the flag "--incompatible_package_name_is_a_function"

https://docs.bazel.build/versions/master/skylark/backward-compatibility.html#package-name-is-a-function

Fixes #5827

RELNOTES: --incompatible_package_name_is_a_fu...

***

PiperOrigin-RevId: 223343999

bazel-io pushed a commit that referenced this issue Nov 30, 2018

Release 0.20.0 (2018-11-30)
Baseline: 7bf7f03

Cherry picks:

   + fd52341:
     update bazel-toolchains pin to latest release Part of changes to
     allow bazelci to use 0.19.0 configs. RBE toolchain configs at or
     before 0.17.0 are not compatible with bazel 0.19.0 or above.
   + 241f28d:
     Revert "Toggle --incompatible_disable_late_bound_option_defaults
     flag."
   + f7e5aef:
     Add cc_toolchain targets for the new entries in the default
     cc_toolchain_suite.
   + d2920e3:
     Revert "WindowsFileSystem: open files with delete-sharing"

[Breaking changes in 0.20](https://github.com/bazelbuild/bazel/issues?q=is%3Aissue+label%3Abreaking-change-0.20)

  - [--incompatible_remove_native_http_archive](#6570).
  - [--incompatible_remove_native_git_repository](#6569).
  - [--incompatible_disable_cc_toolchain_label_from_crosstool_proto](#6434).
  - [--incompatible_disable_depset_in_cc_user_flags](#6384).
  - [--incompatible_disable_cc_configuration_make_variables](#6381).
  - [--incompatible_disallow_conflicting_providers](#5902).
  - [--incompatible_range_type](#5264).

[0.20 is a migration window for the following changes](https://github.com/bazelbuild/bazel/issues?q=is%3Aissue+label%3Amigration-0.20)

  - [--incompatible_use_jdk10_as_host_javabase](#6661)
  - [--incompatible_use_remotejdk_as_host_javabase](#6656)
  - [--incompatible_disable_sysroot_from_configuration](#6565)
  - [--incompatible_provide_cc_toolchain_info_from_cc_toolchain_suite](#6537)
  - [--incompatible_disable_depset_in_cc_user_flags](#6383)
  - [--incompatible_package_name_is_a_function](#5827)

[Breaking changes in the next release (0.21)](https://github.com/bazelbuild/bazel/issues?q=is%3Aissue+label%3Abreaking-change-0.21)

  - [--incompatible_use_jdk10_as_host_javabase](#6661)
  - [--incompatible_use_remotejdk_as_host_javabase](#6656)
  - [--incompatible_disable_sysroot_from_configuration](#6565)
  - [--incompatible_provide_cc_toolchain_info_from_cc_toolchain_suite](#6537)
  - [--incompatible_disable_depset_in_cc_user_flags](#6383)
  - [--incompatible_disallow_data_transition](#6153)
  - [--incompatible_package_name_is_a_function](#5827)
  - [--incompatible_disallow_slash_operator](#5823)
  - [--incompatible_static_name_resolution](#5637)

Incompatible changes:

  - the --experimental_no_dotd_scanning_with_modules command line
    argument is not supported anymore.
  - The --prune_cpp_modules command line option is not supported
    anymore.
  - the --experimental_prune_cpp_input_discovery command line option
    is not supported anymore.

New features:

  - Added support for Android NDK r18.

Important changes:

  - The 'default' parameter of attr.output and attr.output_list is
    removed. This is controlled by
    --incompatible_no_output_attr_default
  - A number of platform-related Starlark APIs which were previously
    marked "experimental" are now disabled by default, and may be
    enabled via --experimental_platforms_api
  - Make legacy-test-support ("legacy_test-<api-level>") from
    android_sdk_repository neverlink. The legacy test support
    libraries shouldn't be built into test binaries. To make them
    available at runtime, developers should declare them via
    uses-library:
    https://developer.android.com/training/testing/set-up-project#andr
    oid-test-base
  - query remote server Capabilities (per REAPI v2)
  - CppRules: All cc_toolchains depended on from
    cc_toolchain_suite.toolchains are now analyzed when not using
    platforms in order to select the right cc_toolchain.
  - removed obsolete --explicit_jre_deps flag.
  - Incompatible flag
    --incompatible_disable_legacy_cpp_toolchain_skylark_api was
    flipped.
  - Improve error messaging when unsupport proguard options are
    specified at the library level.
  - Incompatible flag
    --incompatible_disable_legacy_cpp_toolchain_skylark_api was
    flipped.
  - Incompatible flag
    --incompatible_disable_legacy_cpp_toolchain_skylark_api was
    flipped.
  - The --incompatible_disable_late_bound_option_defaults flag has
    been flipped (#6384)
  - Incompatible flag
    --incompatible_disable_legacy_flags_cc_toolchain_api was flipped
    (#6434)
  - Fixed issue where ctx.resolve_command created conflicting
    intermediate files when resolve_command was called multiple times
    within the same rule invocation with a long command attribute.
  - Incompatible flag
    --incompatible_disable_cc_configuration_make_variables was
    flipped (#6381)
  - If the --javabase flag is unset, it Bazel locates a JDK using
    the JAVA_HOME environment variable and searching the PATH. If no
    JDK is found --javabase will be empty, and builds targeting Java
    will not
    be supported. Previously Bazel would fall back to using the
    embedded
    JDK as a --javabase, but this is no longer default behaviour. A
    JDK should
    be explicitly installed instead to enable Java development
  - Bazel will now shut down when idle for 5 minutes and the system
    is low on RAM (linux only).
  - CROSSTOOL file is now read from the package of cc_toolchain, not
    from
    the package of cc_toolchain_suite. This is not expected to break
    anybody since
    cc_toolchain_suite and cc_toolchain are commonly in the same
    package.
  - All overrides of Starlark's ctx.new_file function are now
    deprecated.
      Try the `--incompatible_new_actions_api` flag to ensure your
    code is forward-compatible.
  - --incompatible_disable_cc_toolchain_label_from_crosstool_proto
    was flipped.
  - Introduce --(no)shutdown_on_low_sys_mem startup flag to toggle
    idle low-memory shutdown, disabled by default.
  - --incompatible_disable_cc_toolchain_label_from_crosstool_proto
    was flipped.
  - --incompatible_disable_cc_toolchain_label_from_crosstool_proto
    was flipped.
  - CppRules: All cc_toolchains depended on from
    cc_toolchain_suite.toolchains are now analyzed when not using
    platforms in order to select the right cc_toolchain.
  - The function `attr.license` is deprecated and will be removed.
      It can be disabled now with `--incompatible_no_attr_license`.
  - `range()` function now returns a lazy value
    (`--incompatible_range_type` is now set by default).
  - The code coverage report now includes the actual paths to header
    files instead of the ugly,
    Bazel generated, virtual includes path.
  - `--incompatible_disallow_conflicting_providers` has been switched
    to true
  - Add new flag `--incompatible_disable_systool_from_configration` to
    disable loading the systool from CppConfiguration.
  - Add new flag `--incompatible_disable_sysroot_from_configuration`
    to
    disable loading the systool from CppConfiguration.
  - Sorting remote Platform properties for remote execution. May
    affect cache keys!
  - Use different server log files per Bazel server process; java.log
    is
    now a symlink to the latest log.

This release contains contributions from many people at Google, as well as a7g4 <a7g4@a7g4.net>, Alan <alan.agius@betssongroup.com>, Asaf Flescher <asafflesch@gmail.com>, Benjamin Peterson <bp@benjamin.pe>, Ed Schouten <ed.schouten@prodrive-technologies.com>, George Gensure <ggensure@uber.com>, George Kalpakas <kalpakas.g@gmail.com>, Greg <gregestren@users.noreply.github.com>, Irina Iancu <iirina@users.noreply.github.com>, Keith Smiley <keithbsmiley@gmail.com>, Loo Rong Jie <loorongjie@gmail.com>, Mark Zeren <mzeren@vmware.com>, Petros Eskinder <petroseskinder@users.noreply.github.com>, rachcatch <rachelcatchpoole@hotmail.com>, Robert Brown <robert.brown@gmail.com>, Robert Gay <robert.gay@redfin.com>, Salty Egg <2281521+zhouhao@users.noreply.github.com>.

ahirreddy pushed a commit to ahirreddy/bazel that referenced this issue Dec 1, 2018

Release 0.20.0 (2018-11-30)
Baseline: 7bf7f03

Cherry picks:

   + fd52341:
     update bazel-toolchains pin to latest release Part of changes to
     allow bazelci to use 0.19.0 configs. RBE toolchain configs at or
     before 0.17.0 are not compatible with bazel 0.19.0 or above.
   + 241f28d:
     Revert "Toggle --incompatible_disable_late_bound_option_defaults
     flag."
   + f7e5aef:
     Add cc_toolchain targets for the new entries in the default
     cc_toolchain_suite.
   + d2920e3:
     Revert "WindowsFileSystem: open files with delete-sharing"

[Breaking changes in 0.20](https://github.com/bazelbuild/bazel/issues?q=is%3Aissue+label%3Abreaking-change-0.20)

  - [--incompatible_remove_native_http_archive](bazelbuild#6570).
  - [--incompatible_remove_native_git_repository](bazelbuild#6569).
  - [--incompatible_disable_cc_toolchain_label_from_crosstool_proto](bazelbuild#6434).
  - [--incompatible_disable_depset_in_cc_user_flags](bazelbuild#6384).
  - [--incompatible_disable_cc_configuration_make_variables](bazelbuild#6381).
  - [--incompatible_disallow_conflicting_providers](bazelbuild#5902).
  - [--incompatible_range_type](bazelbuild#5264).

[0.20 is a migration window for the following changes](https://github.com/bazelbuild/bazel/issues?q=is%3Aissue+label%3Amigration-0.20)

  - [--incompatible_use_jdk10_as_host_javabase](bazelbuild#6661)
  - [--incompatible_use_remotejdk_as_host_javabase](bazelbuild#6656)
  - [--incompatible_disable_sysroot_from_configuration](bazelbuild#6565)
  - [--incompatible_provide_cc_toolchain_info_from_cc_toolchain_suite](bazelbuild#6537)
  - [--incompatible_disable_depset_in_cc_user_flags](bazelbuild#6383)
  - [--incompatible_package_name_is_a_function](bazelbuild#5827)

[Breaking changes in the next release (0.21)](https://github.com/bazelbuild/bazel/issues?q=is%3Aissue+label%3Abreaking-change-0.21)

  - [--incompatible_use_jdk10_as_host_javabase](bazelbuild#6661)
  - [--incompatible_use_remotejdk_as_host_javabase](bazelbuild#6656)
  - [--incompatible_disable_sysroot_from_configuration](bazelbuild#6565)
  - [--incompatible_provide_cc_toolchain_info_from_cc_toolchain_suite](bazelbuild#6537)
  - [--incompatible_disable_depset_in_cc_user_flags](bazelbuild#6383)
  - [--incompatible_disallow_data_transition](bazelbuild#6153)
  - [--incompatible_package_name_is_a_function](bazelbuild#5827)
  - [--incompatible_disallow_slash_operator](bazelbuild#5823)
  - [--incompatible_static_name_resolution](bazelbuild#5637)

Incompatible changes:

  - the --experimental_no_dotd_scanning_with_modules command line
    argument is not supported anymore.
  - The --prune_cpp_modules command line option is not supported
    anymore.
  - the --experimental_prune_cpp_input_discovery command line option
    is not supported anymore.

New features:

  - Added support for Android NDK r18.

Important changes:

  - The 'default' parameter of attr.output and attr.output_list is
    removed. This is controlled by
    --incompatible_no_output_attr_default
  - A number of platform-related Starlark APIs which were previously
    marked "experimental" are now disabled by default, and may be
    enabled via --experimental_platforms_api
  - Make legacy-test-support ("legacy_test-<api-level>") from
    android_sdk_repository neverlink. The legacy test support
    libraries shouldn't be built into test binaries. To make them
    available at runtime, developers should declare them via
    uses-library:
    https://developer.android.com/training/testing/set-up-project#andr
    oid-test-base
  - query remote server Capabilities (per REAPI v2)
  - CppRules: All cc_toolchains depended on from
    cc_toolchain_suite.toolchains are now analyzed when not using
    platforms in order to select the right cc_toolchain.
  - removed obsolete --explicit_jre_deps flag.
  - Incompatible flag
    --incompatible_disable_legacy_cpp_toolchain_skylark_api was
    flipped.
  - Improve error messaging when unsupport proguard options are
    specified at the library level.
  - Incompatible flag
    --incompatible_disable_legacy_cpp_toolchain_skylark_api was
    flipped.
  - Incompatible flag
    --incompatible_disable_legacy_cpp_toolchain_skylark_api was
    flipped.
  - The --incompatible_disable_late_bound_option_defaults flag has
    been flipped (bazelbuild#6384)
  - Incompatible flag
    --incompatible_disable_legacy_flags_cc_toolchain_api was flipped
    (bazelbuild#6434)
  - Fixed issue where ctx.resolve_command created conflicting
    intermediate files when resolve_command was called multiple times
    within the same rule invocation with a long command attribute.
  - Incompatible flag
    --incompatible_disable_cc_configuration_make_variables was
    flipped (bazelbuild#6381)
  - If the --javabase flag is unset, it Bazel locates a JDK using
    the JAVA_HOME environment variable and searching the PATH. If no
    JDK is found --javabase will be empty, and builds targeting Java
    will not
    be supported. Previously Bazel would fall back to using the
    embedded
    JDK as a --javabase, but this is no longer default behaviour. A
    JDK should
    be explicitly installed instead to enable Java development
  - Bazel will now shut down when idle for 5 minutes and the system
    is low on RAM (linux only).
  - CROSSTOOL file is now read from the package of cc_toolchain, not
    from
    the package of cc_toolchain_suite. This is not expected to break
    anybody since
    cc_toolchain_suite and cc_toolchain are commonly in the same
    package.
  - All overrides of Starlark's ctx.new_file function are now
    deprecated.
      Try the `--incompatible_new_actions_api` flag to ensure your
    code is forward-compatible.
  - --incompatible_disable_cc_toolchain_label_from_crosstool_proto
    was flipped.
  - Introduce --(no)shutdown_on_low_sys_mem startup flag to toggle
    idle low-memory shutdown, disabled by default.
  - --incompatible_disable_cc_toolchain_label_from_crosstool_proto
    was flipped.
  - --incompatible_disable_cc_toolchain_label_from_crosstool_proto
    was flipped.
  - CppRules: All cc_toolchains depended on from
    cc_toolchain_suite.toolchains are now analyzed when not using
    platforms in order to select the right cc_toolchain.
  - The function `attr.license` is deprecated and will be removed.
      It can be disabled now with `--incompatible_no_attr_license`.
  - `range()` function now returns a lazy value
    (`--incompatible_range_type` is now set by default).
  - The code coverage report now includes the actual paths to header
    files instead of the ugly,
    Bazel generated, virtual includes path.
  - `--incompatible_disallow_conflicting_providers` has been switched
    to true
  - Add new flag `--incompatible_disable_systool_from_configration` to
    disable loading the systool from CppConfiguration.
  - Add new flag `--incompatible_disable_sysroot_from_configuration`
    to
    disable loading the systool from CppConfiguration.
  - Sorting remote Platform properties for remote execution. May
    affect cache keys!
  - Use different server log files per Bazel server process; java.log
    is
    now a symlink to the latest log.

This release contains contributions from many people at Google, as well as a7g4 <a7g4@a7g4.net>, Alan <alan.agius@betssongroup.com>, Asaf Flescher <asafflesch@gmail.com>, Benjamin Peterson <bp@benjamin.pe>, Ed Schouten <ed.schouten@prodrive-technologies.com>, George Gensure <ggensure@uber.com>, George Kalpakas <kalpakas.g@gmail.com>, Greg <gregestren@users.noreply.github.com>, Irina Iancu <iirina@users.noreply.github.com>, Keith Smiley <keithbsmiley@gmail.com>, Loo Rong Jie <loorongjie@gmail.com>, Mark Zeren <mzeren@vmware.com>, Petros Eskinder <petroseskinder@users.noreply.github.com>, rachcatch <rachelcatchpoole@hotmail.com>, Robert Brown <robert.brown@gmail.com>, Robert Gay <robert.gay@redfin.com>, Salty Egg <2281521+zhouhao@users.noreply.github.com>.

bazel-io pushed a commit that referenced this issue Dec 5, 2018

Code simplification in the environment, remove old incompatible flags
Remove --incompatible_static_name_resolution and --incompatible_package_name_is_a_function

#5827
#5637

RELNOTES: None.
PiperOrigin-RevId: 224140026
@jin

This comment has been minimized.

Copy link
Member

jin commented Dec 5, 2018

@laurentlb Protobuf was just tagged with 3.6.1.2 , but the tag has not been promoted to a release yet.. The latest protobuf release is still 3.6.1, which still breaks on this incompatible change. Why is the incompatible flag removed already?

Also: all android_instrumentation_test targets depend on github.com/android/android-test, which is still using Protobuf 3.4.1. However, it's not trivial to just update to 3.6.1.2 (or any later Protobuf version for that matter), because each project may hit different breaking changes within Protobuf upgrades themselves. We currently use the --incompatible_ flag to have a workaround to keep the tests passing (which can reveal other breakages) until the issues are fully resolved.

cc @brettchabot

@laurentlb

This comment has been minimized.

Copy link
Contributor

laurentlb commented Dec 5, 2018

Bazel 0.21 still has the flag. Bazel 0.22 won't have the flag, but it will be released end of January.

@meteorcloudy

This comment has been minimized.

Copy link
Member

meteorcloudy commented Dec 6, 2018

@laurentlb Is it too soon to remove this flag from HEAD? Because if we do this, there's no way we can test Android in downstream. The cannot use this flag as a workaround, nor can they upgrade protobuf (until 3.7 came out). But I'd like to keep Android Testing in downstream to catch other errors.
Does --incompatible_package_name_is_a_function have to be removed in 0.22.0?

@meteorcloudy

This comment has been minimized.

Copy link
Member

meteorcloudy commented Dec 6, 2018

Good news! It turned it's fine to upgrade android_test to protobuf 3.6.1.2. See my PR android/android-test#147 and tested with android testing at googlesamples/android-testing#227

@meteorcloudy

This comment has been minimized.

Copy link
Member

meteorcloudy commented Dec 6, 2018

@jin, can you make sure android/android-test#147 get merged and update the ATS commit in android_testing?

@jin

This comment has been minimized.

Copy link
Member

jin commented Dec 6, 2018

@meteorcloud wow thanks for looking into this! I’ll follow up on your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment