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

More feedback from review of exec platform scoped spawn strategies proposal #367

Merged
merged 6 commits into from
Feb 13, 2024

Conversation

Silic0nS0ldier
Copy link
Contributor

@Silic0nS0ldier Silic0nS0ldier commented Jan 25, 2024

Main changes of significance here is the renaming of 1 flag.

--allowed_strategies_execution_platform to specify an allowlist for an exec platform

  • A little less confusing to read than --execution_platform_supported_strategies.
  • Read as "allowed strategies for execution platform".

The strictness flag --require_platform_scoped_strategies seems a little off but I'm unable to think up a better option at this point in time. I understand these proposals are not gospel so if I (or anyone else) comes up with better names, I'll put them forward with the implementation.

@Silic0nS0ldier
Copy link
Contributor Author

Silic0nS0ldier commented Jan 26, 2024

Existence of --extra_exec_platforms flag does not actually exist, it seems I was going off some incorrect information. I'll get this fixed in the proposal.

EDIT: Done

@katre katre self-requested a review January 29, 2024 21:00
# For "//:linux_amd64" only allow "remote"
build --execution_platform_supported_strategies=//:linux_amd64=remote
# Allow usage of "worker", "sandboxed" or "local" when exec platform is "//:darwin_arm64"
build --allowed_strategies_execution_platform//:darwin_arm64=worker,sandboxed,local
Copy link
Member

Choose a reason for hiding this comment

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

How about --allowed_strategies_by_execution_platform? Or by_exec_platform if it's too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do like making things shorter... Let's see how each is used currently.

exec:

  • --experimental_add_exec_constraints_to_targets, exec constraints.
  • --experimental_exec_config, overriding default cfg = "exec" transition.
  • --experimental_exec_config_diff, debugging the latter.
  • --experimental_exec_configuration_distinguisher, migration/testing of potential action conflicts.
  • --incompatible_auto_exec_groups, automatic creationof exec groups for toolchains.
  • --remote_default_exec_properties, remote-specific value for a platform's exec_properties attribute.
  • --remote_exec_header, specifying a header to include in remote action spawns.

execution:

  • --extra_execution_platforms=... and register_execution_platforms(...), add extra execution platform(s).
  • --dynamic_local_execution_delay, dynamic execution strategy.
  • --execution_log_binary_file, enables binary action execution log.
  • --execution_log_json_file, enables JSON action execution log.
  • --execution_log_sort, controls sorting of the latter 2 (doesn't take much for this to cause an OoM)
  • --experimental_enable_execution_graph_log, not 100% sure what this does but it seems to be related to logging executed actions like the latter 3.
  • --experimental_execution_graph_enable_edges_from_filewrite_actions, looks like a proposed bug fix.
  • --experimental_execution_graph_log_dep_type, seems to control what actions get logged.
  • --experimental_execution_graph_log_middleman, seems to enable logging middleman actions.
  • --experimental_execution_graph_log_path, where data is written.
  • --experimental_execution_graph_log_queue_size, maximum action queue size for logger.
  • --experimental_include_xcode_execution_requirements, adds an execution requirement to XCode actions.
  • --experimental_remote_execution_keepalive, related to remote execution.
  • --modify_execution_info, modifies action execution metadata.
  • --remote_execution_priority, requested relative priority of actions to be executed remotely.
  • --remote_print_execution_messages, controls presentation of remote execution action spawn output.

In the end exec seems to have usage that aligns closely with the scope of this proposal (execution platform selection and customisation), so I think --allowed_strategies_by_exec_platform makes the most sense.

@lberki
Copy link
Contributor

lberki commented Feb 12, 2024

I'll defer to @katre .

@katre katre merged commit 14f45b8 into bazelbuild:main Feb 13, 2024
1 check passed
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.

3 participants