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_strict_action_env #6648

Closed
buchgr opened this issue Nov 10, 2018 · 6 comments
Closed

--incompatible_strict_action_env #6648

buchgr opened this issue Nov 10, 2018 · 6 comments
Assignees
Labels
incompatible-change Incompatible/breaking change

Comments

@buchgr
Copy link
Contributor

buchgr commented Nov 10, 2018

Starting with Bazel 0.21 the --experimental_strict_action_env option has been renamed to --incompatible_strict_action_env and is now on by default. This means Bazel will no longer use the client's PATH and LD_LIBRARY_PATH environmental variables in the default action environment. If the old behavior is desired, pass --action_env=PATH and --action_env=LD_LIBRARY_PATH. --noincompatible_strict_action_env will also temporarily restore the old behavior. However, as --action_env is a more general and explicit way to pass client environmental variables into actions, --noincompatible_strict_action_env will eventually be deprecated and removed. See #6648 for more details.

Both flag names --experimental_strict_action_env and --incompatible_strict_action_env will keep working for a few releases.

Also see #2574

cc: @benjaminp

@buchgr buchgr added this to the Remote Execution milestone Nov 10, 2018
@buchgr buchgr self-assigned this Nov 11, 2018
@buchgr buchgr added the incompatible-change Incompatible/breaking change label Nov 12, 2018
@benjaminp
Copy link
Collaborator

Do you need to me to update the PR somehow?

@buchgr
Copy link
Contributor Author

buchgr commented Nov 12, 2018

No I did the update already. Just had to submit a different change before submitting your change. should be in in a few hours.

bazel-io pushed a commit that referenced this issue Nov 19, 2018
Also rename it to --incompatible_strict_action_env. See #6648 and #2574. For a while, many actions did not respect the --action_env command line option, but those have been fixed now. So, I think it's time to flip on this flag for greater hermeticity by default.

Closes #6263.

RELNOTES: The --experimental_strict_action_env option has been renamed to --incompatible_strict_action_env and is now on by default. This means Bazel will no longer use the client's PATH and LD_LIBRARY_PATH environmental variables in the default action environment. If the old behavior is desired, pass --action_env=PATH and --action_env=LD_LIBRARY_PATH. --noincompatible_strict_action_env will also temporarily restore the old behavior. However, as --action_env is a more general and explicit way to pass client environmental variables into actions, --noincompatible_strict_action_env will eventually be deprecated and removed. See #6648 for more details.
PiperOrigin-RevId: 222053105
bazel-io pushed a commit that referenced this issue Dec 19, 2018
Baseline: cb9b2af

Cherry picks:

   + 12b9646:
     Windows, test wrapper: rename the associated flag
   + 7fc967c:
     Use a fixed thread pool in ByteStreamBuildEventArtifactUploader
   + 798b9a9:
     Add --build_event_upload_max_threads option
   + dbe05df:
     Update the version of  skylib bundled in the distfile

Incompatible changes:

  - The --experimental_stl command line option is removed.
  - aquery defaults to human readable output format.

New features:

  - repository_ctx.download and repository_ctx.download_and_extract
    now return a struct.
  - Android Databinding v2 can be enabled with
    --experimental_android_databinding_v2.

Important changes:

  - The deprecated and unmaintained Docker rules in
    tools/build_defs/docker were removed. Please use
    https://github.com/bazelbuild/rules_docker instead.
  - The new --upload_query_output_using_bep query/cquery/aquery flag
    causes query outputs to be uploaded via BEP.
  - New incompatible flag --incompatible_strict_argument_ordering
  - --strict_android_deps and --strict_java_deps were renamed to
    --experimental_strict_java_deps
  - config_settings that select on "compiler" value instead of values
    = {"compiler" : "x"} should use flag_values =
    {"@bazel_tools//tools/cpp:compiler": "x"}.
  - The new --upload_query_output_using_bep query/cquery/aquery flag
    causes query outputs to be uploaded via BEP.
  - Turn on --incompatible_disable_sysroot_from_configuration
  - We revamped our Android with Bazel tutorial! Check it out
    [here](https://docs.bazel.build/versions/master/tutorial/android-a
    pp.html).
  - --incompatible_disallow_slash_operator is now on by default
  - Enable --experimental_check_desugar_deps by default.  This flag
    rules out several types of invalid Android builds at compile-time.
  - The --max_config_changes_to_show option lists the names of
    options which
    have changed and thus caused the analysis cache to be dropped.
  - The --experimental_strict_action_env option has been renamed to
    --incompatible_strict_action_env and is now on by default. This
    means Bazel will no longer use the client's PATH and
    LD_LIBRARY_PATH environmental variables in the default action
    environment. If the old behavior is desired, pass
    --action_env=PATH and --action_env=LD_LIBRARY_PATH.
    --noincompatible_strict_action_env will also temporarily restore
    the old behavior. However, as --action_env is a more general and
    explicit way to pass client environmental variables into actions,
    --noincompatible_strict_action_env will eventually be deprecated
    and removed. See #6648 for more details.
  - XCRUNWRAPPER_LABEL has been removed. If you used this value
    before, please use @bazel_tools//tools/objc:xcrunwrapper instead.
  - --incompatible_static_name_resolution is no unable by default
  - We will phase out --genrule_strategy in favor of
    --strategy=Genrule=<value> (for genrules) or
    --spawn_strategy=<value> (for all actions).
  - --incompatible_package_name_is_a_function is now enabled by
    default
  - Dynamic execution is now available with
    --experimental_spawn_strategy. Dynamic execution allows a build
    action to run locally and remotely simultaneously, and Bazel
    picks the fastest action. This provides the best of both worlds:
    faster clean builds than pure local builds, and faster
    incremental builds than pure remote builds.
  - --incompatible_package_name_is_a_function is now enabled by
    default
  - New incompatible flag --incompatible_merge_genfiles_directory
  - grpc log now logs updateActionResult
  - CppConfiguration doesn't do package loading anymore. That means:
    * it's no longer needed to have C++ toolchain available when
    building non-C++ projects
    * bazel will not analyze C++ toolchain when not needed -> speedup
    ~2s on bazel startup when C++ rules using hermetic toolchain are
    not loaded
  - --incompatible_package_name_is_a_fu...

This release contains contributions from many people at Google, as well as andy g scott ?, Attila Ol?h, Benjamin Peterson, Clint Harrison, Dave Lee, Ed Schouten, Greg Estren, Gregor Jasny, Jamie Snape, Jerry Marino, Loo Rong Jie, Or Shachar, Sevki Hasirci, William Chargin.
wchargin added a commit to tensorflow/tensorboard that referenced this issue Dec 21, 2018
Summary:
Bazel 0.21.0 includes a breaking change to our transitive dependencies
through TensorFlow. TensorFlow has patched its workspace file
accordingly. This commit upgrades our pinned version of the TensorFlow
workspace to accommodate.

Issue #1710 remains open after this change, because Bazel 0.21.0 has a
further breaking change that causes our tests to fail when run with the
default Bazel flags (see test plan below):
<bazelbuild/bazel#6648>.

Test Plan:
On Bazel 0.21.0, running `bazel query 'deps(//...)'` fails before this
change:

    The value 'REPOSITORY_NAME' has been removed in favor of 'repository_name()', please use the latter (https://docs.bazel.build/versions/master/skylark/lib/native.html#repository_name). You can temporarily allow the old name by using --incompatible_package_name_is_a_function=false
    ERROR: Evaluation of query "deps(//tensorboard)" failed: errors were encountered while computing transitive closure

…but passes after it.

Also, `bazel test --incompatible_strict_action_env=false //...` still
passes on nightly. The new flag is needed due to a breaking change in
Bazel 0.21.0 that this commit does not address.

wchargin-branch: deps-tf-workspace-20181220
wchargin added a commit to tensorflow/tensorboard that referenced this issue Dec 21, 2018
Summary:
Bazel 0.21.0 includes a breaking change to our transitive dependencies
through TensorFlow. TensorFlow has patched its workspace file
accordingly. This commit upgrades our pinned version of the TensorFlow
workspace to accommodate.

Issue #1710 remains open after this change, because Bazel 0.21.0 has a
further breaking change that causes our tests to fail when run with the
default Bazel flags (see test plan below):
<bazelbuild/bazel#6648>.

Test Plan:
On Bazel 0.21.0, running `bazel query 'deps(//...)'` fails before this
change:

    The value 'REPOSITORY_NAME' has been removed in favor of 'repository_name()', please use the latter (https://docs.bazel.build/versions/master/skylark/lib/native.html#repository_name). You can temporarily allow the old name by using --incompatible_package_name_is_a_function=false
    ERROR: Evaluation of query "deps(//tensorboard)" failed: errors were encountered while computing transitive closure

…but passes after it.

Also, `bazel test --incompatible_strict_action_env=false //...` still
passes on nightly. The new flag is needed due to a breaking change in
Bazel 0.21.0 that this commit does not address.

wchargin-branch: deps-tf-workspace-20181220
lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Sep 20, 2019
'--experimental_strict_action_env' was a Bazel 0.20.0 option which got
renamed to '--incompatible_strict_action_env' and was planned to be the
default as of 0.21.0 [1]. However, this planned flip still hasn't
happened for at least the baseline of 1.0.0 (as of rc3), so we can't
remove it as of now [2].

In general, we should try to avoid having --incompatible_* or
--experimental_* flags in the bazelrc, as per explanation in Issue 11122
[3] as well as on the 'Backward Compatibility' page of Bazel [4]:

> Users should never run their production builds with --experimental_*
> or --incompatible_* flags.

However, for this case we have a compelling reason to have
'--incompatible_strict_action_env'.

[1]: bazelbuild/bazel#6648
[2]: bazelbuild/bazel#7026
[3]: https://bugs.chromium.org/p/gerrit/issues/detail?id=11122
[4]: https://docs.bazel.build/versions/0.29.1/backward-compatibility.html

Change-Id: Ie78708ffaeb1bfe9ebceb924939833f7c30eaeaf
@mattgodbolt
Copy link

I checked bazel v1.1.0 and it seems this flag is still off by default:

~/d/research (master|✔) 28.1s $ bazel version
Bazelisk version: v1.1.0
Build label: 1.1.0
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Mon Oct 21 08:44:00 2019 (1571647440)
Build timestamp: 1571647440
Build timestamp as int: 1571647440
~/d/research (master|✔) 1.1s $ bazel help build | grep incompatible_strict_action
  --[no]incompatible_strict_action_env (a boolean; default: "false")

Is this as intended?

@benjaminp
Copy link
Collaborator

#7026

@mattgodbolt
Copy link

Thanks @benjaminp :)

@BoleynSu
Copy link
Contributor

Hi @buchgr , the default is currently false instead of true, would you mind updating your comments accordingly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change
Projects
None yet
Development

No branches or pull requests

5 participants