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

feat: expose a config_setting for copy execution_requirements #606

Merged
merged 4 commits into from Oct 9, 2023

Conversation

alexeagle
Copy link
Member

@alexeagle alexeagle commented Oct 9, 2023

Alternative to #605

Fixes #604
Fixes #466


Type of change

  • New feature or functionality (change which adds functionality)

For changes visible to end-users

  • Suggested release notes are provided below:

If you rely on Remote Execution and Build-without-the-bytes, you should add to your .bazelrc:

(with Bazel 6.4 or greater)
common --@aspect_bazel_lib//lib:copy_use_local_execution=false

or with Bazel 6.3 or earlier:

build --@aspect_bazel_lib//lib:copy_use_local_execution=false
fetch --@aspect_bazel_lib//lib:copy_use_local_execution=false
query --@aspect_bazel_lib//lib:copy_use_local_execution=false

Test plan

  • Manual testing; please provide instructions so we can reproduce:
    Run aquery locally:
$ bazel aquery lib/tests/copy_file/...
action 'Copying file lib/tests/copy_file/file1'
  Mnemonic: CopyFile
  Target: //lib/tests/copy_file:copy_same_file_b
  Configuration: k8-opt
  Execution platform: @local_config_platform//:host
  ActionKey: 51a4a2f509cd84b088d8a06fdd9136b87a9dfc494dbdeb31547395af412881cf
  Inputs: [lib/tests/copy_file/file1]
  Outputs: [bazel-out/k8-opt/bin/lib/tests/copy_file/file1_copy_b]
  Environment: [PATH=/bin:/usr/bin:/usr/local/bin]
  ExecutionInfo: {local: 1, no-cache: 1, no-remote: 1, no-remote-cache: 1, no-remote-exec: 1, no-sandbox: 1}
  Command Line: (exec /bin/bash \
    -c \
    'cp -f "$1" "$2"' \
    '' \
    lib/tests/copy_file/file1 \
    bazel-out/k8-opt/bin/lib/tests/copy_file/file1_copy_b)
# Configuration: c5a436e84a36b625a7552b0800bf8d2dcbfb101629fb9f434c99131217b14a76
# Execution platform: @local_config_platform//:host
  ExecutionInfo: {local: 1, no-cache: 1, no-remote: 1, no-remote-cache: 1, no-remote-exec: 1, no-sandbox: 1}

$ bazel aquery lib/tests/copy_file/... --@aspect_bazel_lib//lib:copy_use_local_execution=false
action 'Copying file lib/tests/copy_file/file1'
  Mnemonic: CopyFile
  Target: //lib/tests/copy_file:copy_same_file_b
  Configuration: k8-opt
  Execution platform: @local_config_platform//:host
  ActionKey: 47db716e92e1dc33fbf00aee500b199acc5c525d3f2a471afa6d2e4717179469
  Inputs: [lib/tests/copy_file/file1]
  Outputs: [bazel-out/k8-opt/bin/lib/tests/copy_file/file1_copy_b]
  Environment: [PATH=/bin:/usr/bin:/usr/local/bin]
  Command Line: (exec /bin/bash \
    -c \
    'cp -f "$1" "$2"' \
    '' \
    lib/tests/copy_file/file1 \
    bazel-out/k8-opt/bin/lib/tests/copy_file/file1_copy_b)
# Configuration: 8c0100f9bf1701776d7be2368f93477816cb926fec2e88b2e55ce87346ce20df
# Execution platform: @local_config_platform//:host



@alexeagle alexeagle marked this pull request as ready for review October 9, 2023 19:58
Comment on lines 6 to 7
NB: if you use Remote Execution and Build-without-the-bytes, then you'll want the copy action to
occur on the remote machine. You should therefore disable our `copy_use_local_execution` flag
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Could you give one more tidbit about why they want the copy action to occur on the remote machine? Is it just to avoid some warnings?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/private/copy_common.bzl Show resolved Hide resolved
Comment on lines 6 to 7
NB: if you use Remote Execution and Build-without-the-bytes, then you'll want the copy action to
occur on the remote machine. You should therefore disable our `copy_use_local_execution` flag
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexeagle alexeagle merged commit 4bfe557 into 2.x Oct 9, 2023
42 checks passed
@alexeagle alexeagle deleted the exec_reqts branch October 9, 2023 20:57
alexeagle added a commit that referenced this pull request Oct 9, 2023
* feat: expose a config_setting for copy execution_requirements

Fixes #604

* chore: add user docs

* chore: improve docs

* chore: better link to copy_file
alexeagle added a commit that referenced this pull request Oct 9, 2023
* feat: expose a config_setting for copy execution_requirements

Fixes #604

* chore: add user docs

* chore: improve docs

* chore: better link to copy_file
alexeagle added a commit that referenced this pull request Oct 9, 2023
* feat: expose a config_setting for copy execution_requirements

Fixes #604

* chore: add user docs

* chore: improve docs

* chore: better link to copy_file
alexeagle added a commit that referenced this pull request Oct 9, 2023
…607)

* feat: expose a config_setting for copy execution_requirements

Fixes #604

* chore: add user docs

* chore: improve docs

* chore: better link to copy_file
alexeagle added a commit that referenced this pull request Dec 23, 2023
* feat: expose a config_setting for copy execution_requirements

Fixes #604

* chore: add user docs

* chore: improve docs

* chore: better link to copy_file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants