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

incompatible_disallow_hashing_frozen_mutables: freezing a list should not make it hashable #7800

Closed
laurentlb opened this issue Mar 21, 2019 · 4 comments

Comments

@laurentlb
Copy link
Member

@laurentlb laurentlb commented Mar 21, 2019

Reported by @alandonovan

$ cat test/inc.bzl 
a_list = []

$ cat test/BUILD 
load(":inc.bzl", "a_list")

print(a_list)
print({a_list: 1}) # ok
print({[]: 1}) # error

$ bazel query test:*
DEBUG: test/BUILD:3:1: [].
DEBUG: test/BUILD:4:1: {[]: 1}.
ERROR: test/BUILD:5:7: Traceback (most recent call last):
        File "test/BUILD", line 5
                print({[]: 1})
        File "test/BUILD", line 5, in print
                {[]: 1}
unhashable type: 'list'.

Lists shouldn't be hashable, even if they are frozen.
For the user, a possible fix is to convert the list to a tuple.

@alandonovan

This comment has been minimized.

Copy link

@alandonovan alandonovan commented Mar 21, 2019

Besides the slightly strange observation that freezing a value both removes some operations (mutation) while adding others (hashability), a reason not to allow this is that a mutable value may contain itself, such as a list that contains itself as an element. Without major changes to the implementation of hashing, attempting to hash such a value would get stuck or crash.

Reliance on this feature was already removed from all of Google's loading-phase Starlark with very modest effort. I imagine the challenge will be the analysis phase, where it's common for users to insert struct/provider instances into dicts or depsets. Those structs/providers would have to use tuples instead of lists.

@laurentlb

This comment has been minimized.

Copy link
Member Author

@laurentlb laurentlb commented Jul 24, 2019

cc @Quarz0

Quarz0 added a commit to Quarz0/bazel that referenced this issue Jul 25, 2019
When this flag is enabled, freezing a mutable object will no longer make
it hashable.

RELNOTES: Flag `--incompatible_disallow_hashing_frozen_mutables` is
added. See bazelbuild#7800
bazel-io pushed a commit that referenced this issue Aug 1, 2019
Related: #7800

Closes #8980.

PiperOrigin-RevId: 261150154
brandjon added a commit that referenced this issue Aug 1, 2019
Related: #7800

Closes #8980.

PiperOrigin-RevId: 261150154
@laurentlb laurentlb changed the title freezing a list should not make it hashable incompatible_disallow_hashing_frozen_mutables: freezing a list should not make it hashable Aug 2, 2019
@katre katre added migration-0.29 and removed migration-ready labels Aug 2, 2019
@dslomov

This comment has been minimized.

Copy link
Contributor

@dslomov dslomov commented Aug 5, 2019

What is the extent of the breakage? I assume there is no migration tooling that we can possibly create.

@laurentlb

This comment has been minimized.

Copy link
Member Author

@laurentlb laurentlb commented Aug 5, 2019

Breakages are expected to be rare. I've built large projects inside Google without finding any issue (although Alan says he fixed some in the loading phase).

@bazel-io bazel-io closed this in f01b863 Aug 21, 2019
bazel-io pushed a commit that referenced this issue Oct 10, 2019
Baseline: 97a8264

Cherry picks:

   + a0e3bb2:
     Remove support for authentication and .netrc
   + ada2c55:
     Add explicit --sdk argument to xcrun calls
   + 847df72:
     toolchain_vanilla: Unset source and target language level
     versions
   + 5cfa030:
     Update java_tools version to javac11-v5.1.

Incompatible changes:

  - Python, Windows: the
    --[no]incompatible_windows_escape_python_args is no longer
    supported. (It was flipped to true in Bazel 0.27.0)
  - --incompatible_use_native_patch is enabled by default
  - Windows: --incompatible_windows_bashless_run_command is now true
    by default, meaning "bazel run //foo:bin" will run the binary as
    a subprocess of the Bazel client. (When the flag is false, the
    binary is executed as a subprocess of Bash.)
  - Windows: --incompatible_windows_native_test_wrapper is enabled by
    default

New features:

  - Genrule now supports `cmd_bash`, `cmd_ps`, `cmd_bat` attributes.
    More details at
    https://docs.bazel.build/versions/master/be/general.html#genrule.c
    md
  - config_setting can now check multiple values on "--foo=firstVal
    --foo=secondVal ..."-style flags
  - tags: use `--experimental_allow_tags_propagation` flag to
    propagate tags to the action's execution requirements from
    targets. Such tags should start with: `no-`, `requires-`,
    `supports-`, `block-`, `disable-`, `cpu:`. See #8830 for details.
  - Users can now get generated def file from cc_library via
    "def_file" output group on Windows.
  - Platform-specific bazelrc: with --enable_platform_specific_config
    you can
    enable flags in bazelrc according to your host platform.
  - tags: use `--experimental_allow_tags_propagation` flag to
    propagate tags to the action's execution requirements from
    cc_library or cc_binary targets. Such tags should start with:
    `no-`, `requires-`, `supports-`, `block-`, `disable-`, `cpu:`.
    See #8830 for details.
  - tags: use --experimental_allow_tags_propagation flag to propagate
    tags to the action's execution requirements from java targets.
    Such tags should start with: no-, requires-, supports-, block-,
    disable-, cpu:. See #8830 for details.

Important changes:

  - Bazel Android builds now use aapt2 by default. To revert to aapt,
    set `--android_aapt=aapt`.
  - Make either --subcommands or --verbose_failures imply
    --materialize_param_files
  - Bazel Android builds now use aapt2 by default. To revert to aapt,
    set `--an...
    RELNOTES: None
  - by default all remote connections considered to be via `gRPC`
    with TLS enabled, unless other specified. To disable TLS use
    `grpc://` prefix for you endpoints. All remote connections via
    `gRPC` affected - `--remote_cache`, `--remote_executor` or
    `--bes_backend`. http cache/executor is not affected. See #8061
    for details.
  - cc_* rules support non-transitive defines through a
    'local_defines' attribute.
  - Enable
    incompatible_disallow_rule_execution_platform_constraints_allowed
    by default (#8136).
  - incompatible_disallow_split_empty_separator is enabled by default
  - Fixed Android build issues with aapt2 on Windows. See the [GitHub
    issue](#9102) for more
    information.
  - --incompatible_disable_static_cc_toolchains has been flipped. See
    #8546.
  - --remote_default_platform_properties has been deprecated in favor
    of --remote_default_exec_properties.
  - The --incompatible_make_thinlto_command_lines_standalone flag has
    been flipped, see #6791
    for more information.
  - The --incompatible_use_specific_tool_files flag has been flipped.
    See #9126 for more
    information.
  - Clarify default visibility.
  - Enables incompatible_auto_configure_host_platform
  - New incompatible flag --incompatible_disable_depset_items
    disables the "items" parameter in the Starlark depset
    constructor. Use "direct" and "transitive" parameters instead.
  - --incompatible_assignment_identifiers_have_local_scope is enabled
  - incompatible_disable_partition_default_parameter is enabled by
    default ()
  - incompatible_restrict_attribute_names is enabled
    (#6437)
  - The --incompatible_disable_nocopts flag has been flipped. See
    #8546 for more
    information.
  - Deprecated Java-Starlark API java_common.create_provider is
    removed. JavaInfo() legacy args (actions, sources, source_jars,
    use_ijar, java_toolchain, host_javabase) are removed.
  - The flag incompatible_disallow_hashing_frozen_mutables is enabled
    (#7800)
  - `maven_jar` and `maven_server` now disallow using plain HTTP URLs
    without a specified checksum. If you are still using `maven_jar`,
    consider migrating to
    [`rules_jvm_external`](https://github.com/bazelbuild/rules_jvm_ext
    ernal) for transitive dependency management. See
    [#8607](#8607) for more
    information.
  - Added `sha256` and `sha256_src` attributes to `maven_jar`. Please
    consider migrating to SHA-256 as SHA-1 has been deemed
    cryptographically insecure ([https://shattered.io]()). Or, use
    [`rules_jvm_external`](https://github.com/bazelbuild/rules_jvm_ext
    ernal) to manage your transitive Maven dependencies with artifact
    pinning and SHA-256 verification support.
  - introducing per-target exec_properties
  - Bazel now supports ThinLTO builds on Linux for Clang versions >=
    6.0. ThinLTO can be enabled through --features=thin_lto
  - The Target.output_group field in Starlark is removed. Use
    OutputGroupInfo instead. See
    #7949 for details.
  - Make a number of parameters of Starlark builtin functions
    positional-only (as opposed to specifiable by keyword). See
    #8147 for details.
  - incompatible_skip_genfiles_symlink is enabled by default (#8651)
  - Change Pruned events will fire immediately after being checked.
  - --incompatible_remove_legacy_whole_archive has been flipped. See
    #7362 for more
    information

This release contains contributions from many people at Google, as well as Adam Liddell, Alessandro Patti, Arshabh Kumar Agarwal, Artem Pelenitsyn, Artem Zinnatullin, Benjamin Peterson, David Ostrovsky, Emmanuel Goh, Farhim Ferdous, George Gensure, iirina, Keith Smiley, Kiril Videlov, Laurent Le Brun, Mantas Sakalauskas, Marwan Tammam, Matt Mukerjee, panzhongxian, Shachar Anchelovich, Stepan Koltsov, Stephan Wolski, Travis Clarke, Yannic Bonenberger, Yuta Saito.
dslomov added a commit that referenced this issue Oct 11, 2019
Baseline: 97a8264

Cherry picks:

   + a0e3bb2:
     Remove support for authentication and .netrc
   + ada2c55:
     Add explicit --sdk argument to xcrun calls
   + 847df72:
     toolchain_vanilla: Unset source and target language level
     versions
   + 5cfa030:
     Update java_tools version to javac11-v5.1.

Incompatible changes:

  - Python, Windows: the
    --[no]incompatible_windows_escape_python_args is no longer
    supported. (It was flipped to true in Bazel 0.27.0)
  - --incompatible_use_native_patch is enabled by default
  - Windows: --incompatible_windows_bashless_run_command is now true
    by default, meaning "bazel run //foo:bin" will run the binary as
    a subprocess of the Bazel client. (When the flag is false, the
    binary is executed as a subprocess of Bash.)
  - Windows: --incompatible_windows_native_test_wrapper is enabled by
    default

New features:

  - Genrule now supports `cmd_bash`, `cmd_ps`, `cmd_bat` attributes.
    More details at
    https://docs.bazel.build/versions/master/be/general.html#genrule.c
    md
  - config_setting can now check multiple values on "--foo=firstVal
    --foo=secondVal ..."-style flags
  - tags: use `--experimental_allow_tags_propagation` flag to
    propagate tags to the action's execution requirements from
    targets. Such tags should start with: `no-`, `requires-`,
    `supports-`, `block-`, `disable-`, `cpu:`. See #8830 for details.
  - Users can now get generated def file from cc_library via
    "def_file" output group on Windows.
  - Platform-specific bazelrc: with --enable_platform_specific_config
    you can
    enable flags in bazelrc according to your host platform.
  - tags: use `--experimental_allow_tags_propagation` flag to
    propagate tags to the action's execution requirements from
    cc_library or cc_binary targets. Such tags should start with:
    `no-`, `requires-`, `supports-`, `block-`, `disable-`, `cpu:`.
    See #8830 for details.
  - tags: use --experimental_allow_tags_propagation flag to propagate
    tags to the action's execution requirements from java targets.
    Such tags should start with: no-, requires-, supports-, block-,
    disable-, cpu:. See #8830 for details.

Important changes:

  - Bazel Android builds now use aapt2 by default. To revert to aapt,
    set `--android_aapt=aapt`.
  - Make either --subcommands or --verbose_failures imply
    --materialize_param_files
  - Bazel Android builds now use aapt2 by default. To revert to aapt,
    set `--an...
    RELNOTES: None
  - by default all remote connections considered to be via `gRPC`
    with TLS enabled, unless other specified. To disable TLS use
    `grpc://` prefix for you endpoints. All remote connections via
    `gRPC` affected - `--remote_cache`, `--remote_executor` or
    `--bes_backend`. http cache/executor is not affected. See #8061
    for details.
  - cc_* rules support non-transitive defines through a
    'local_defines' attribute.
  - Enable
    incompatible_disallow_rule_execution_platform_constraints_allowed
    by default (#8136).
  - incompatible_disallow_split_empty_separator is enabled by default
  - Fixed Android build issues with aapt2 on Windows. See the [GitHub
    issue](#9102) for more
    information.
  - --incompatible_disable_static_cc_toolchains has been flipped. See
    #8546.
  - --remote_default_platform_properties has been deprecated in favor
    of --remote_default_exec_properties.
  - The --incompatible_make_thinlto_command_lines_standalone flag has
    been flipped, see #6791
    for more information.
  - The --incompatible_use_specific_tool_files flag has been flipped.
    See #9126 for more
    information.
  - Clarify default visibility.
  - Enables incompatible_auto_configure_host_platform
  - New incompatible flag --incompatible_disable_depset_items
    disables the "items" parameter in the Starlark depset
    constructor. Use "direct" and "transitive" parameters instead.
  - --incompatible_assignment_identifiers_have_local_scope is enabled
  - incompatible_disable_partition_default_parameter is enabled by
    default ()
  - incompatible_restrict_attribute_names is enabled
    (#6437)
  - The --incompatible_disable_nocopts flag has been flipped. See
    #8546 for more
    information.
  - Deprecated Java-Starlark API java_common.create_provider is
    removed. JavaInfo() legacy args (actions, sources, source_jars,
    use_ijar, java_toolchain, host_javabase) are removed.
  - The flag incompatible_disallow_hashing_frozen_mutables is enabled
    (#7800)
  - `maven_jar` and `maven_server` now disallow using plain HTTP URLs
    without a specified checksum. If you are still using `maven_jar`,
    consider migrating to
    [`rules_jvm_external`](https://github.com/bazelbuild/rules_jvm_ext
    ernal) for transitive dependency management. See
    [#8607](#8607) for more
    information.
  - Added `sha256` and `sha256_src` attributes to `maven_jar`. Please
    consider migrating to SHA-256 as SHA-1 has been deemed
    cryptographically insecure ([https://shattered.io]()). Or, use
    [`rules_jvm_external`](https://github.com/bazelbuild/rules_jvm_ext
    ernal) to manage your transitive Maven dependencies with artifact
    pinning and SHA-256 verification support.
  - introducing per-target exec_properties
  - Bazel now supports ThinLTO builds on Linux for Clang versions >=
    6.0. ThinLTO can be enabled through --features=thin_lto
  - The Target.output_group field in Starlark is removed. Use
    OutputGroupInfo instead. See
    #7949 for details.
  - Make a number of parameters of Starlark builtin functions
    positional-only (as opposed to specifiable by keyword). See
    #8147 for details.
  - incompatible_skip_genfiles_symlink is enabled by default (#8651)
  - Change Pruned events will fire immediately after being checked.
  - --incompatible_remove_legacy_whole_archive has been flipped. See
    #7362 for more
    information

This release contains contributions from many people at Google, as well as Adam Liddell, Alessandro Patti, Arshabh Kumar Agarwal, Artem Pelenitsyn, Artem Zinnatullin, Benjamin Peterson, David Ostrovsky, Emmanuel Goh, Farhim Ferdous, George Gensure, iirina, Keith Smiley, Kiril Videlov, Laurent Le Brun, Mantas Sakalauskas, Marwan Tammam, Matt Mukerjee, panzhongxian, Shachar Anchelovich, Stepan Koltsov, Stephan Wolski, Travis Clarke, Yannic Bonenberger, Yuta Saito.
bazel-io pushed a commit that referenced this issue Nov 6, 2019
#7800

RELNOTES: None.
PiperOrigin-RevId: 278886996
irengrig added a commit to irengrig/bazel that referenced this issue Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.