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

remove need for --define=EXECUTOR=remote in remote execution #7254

Open
buchgr opened this issue Jan 25, 2019 · 14 comments
Open

remove need for --define=EXECUTOR=remote in remote execution #7254

buchgr opened this issue Jan 25, 2019 · 14 comments
Assignees
Labels
not stale Issues or PRs that are inactive but not considered stale P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request

Comments

@buchgr
Copy link
Contributor

buchgr commented Jan 25, 2019

This is mainly used by Bazel to force compiling singlejar and ijar from source when running actions on a remote execution system that might not be compatible with the precompiled singlejar and ijar versions.

Make this obsolete by defining the right platform constraints.

@buchgr buchgr added type: feature request P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Jan 25, 2019
@ittaiz
Copy link
Member

ittaiz commented Jan 26, 2019 via email

@katre katre self-assigned this Mar 21, 2019
@katre
Copy link
Member

katre commented Mar 21, 2019

Taking a look to see how we can use a config_setting to help with this.

@buchgr
Copy link
Contributor Author

buchgr commented Jun 3, 2019

@katre what's the status of this?

@katre
Copy link
Member

katre commented Jun 3, 2019

@agoulti tested a few variations, and ended up stuck on the fact that what is needed is a config_setting that can detect host vs remote, even when host and remote are identical. No good solution to this yet.

@agoulti
Copy link

agoulti commented Jun 10, 2019

I'd say the problem is the combination of these two factors:

  • Currently the platform definition does not capture enough information to know if the pre-built binary will work on that platform
  • We can't reliably tell if we're on the host platform. Since Bazel doesn't transition platforms properly yet, a common workaround is to set the local platform to be remote, so even if we had a way of checking if the chosen platform is the same as local, we can't rely on this in the current state of the world.

@philwo philwo changed the title remove need for --define=executor=REMOTE in remote execution remove need for --define=EXECUTOR=remote in remote execution Jun 27, 2019
@vincyyou
Copy link

.

@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2.5 years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Mar 16, 2023
@uri-canva
Copy link
Contributor

Is this related to Execution Platforms vs. Strategies?

@fmeum fmeum added not stale Issues or PRs that are inactive but not considered stale and removed stale Issues or PRs that are stale (no activity for 30 days) labels Mar 16, 2023
@katre
Copy link
Member

katre commented Mar 16, 2023

The define would probably be able to be removed if these were unified, yes.

@uri-canva
Copy link
Contributor

Do you know if there's any work being done on that? It's shown as approved currently: https://github.com/bazelbuild/proposals/blob/afa38929278b7c20870125d1d32394955967263f/README.md?plain=1#L72 but the last update is a while ago and I haven't stumbled on things related to it in the code.

@katre
Copy link
Member

katre commented Mar 16, 2023

Sadly not. It's a good idea but it hasn't been prioritized and I don't have enough time to work on it as background.

@Silic0nS0ldier
Copy link
Contributor

Is there sufficient capacity to support a community-driven implementation of this? ("this" being the "Execution Platforms vs. Strategies" proposal, which includes resolving this issue)

@katre
Copy link
Member

katre commented Mar 20, 2023

I'd be happy to support a community-driven implementation. My worry currently is that the previous proposal is fairly old, and could probably use a rewrite to update it and provide clarity with new features.

@Silic0nS0ldier
Copy link
Contributor

Silic0nS0ldier commented Aug 12, 2023

It still needs more work (polish, expanding on background, etc) but since I've got a functional prototype I figured it worth sharing.

bazelbuild/proposals#359

Compared to the Strategy Properties on Platforms proposal;

  1. It does not replace the existing spawn strategy controls.
  2. Existing spawn strategy flags --spawn_strategy (default), --strategy (mnemonic) and --strategy_regexp (regex match action description) are expanded to allow platform targeting.

Source for the prototype is at Silic0nS0ldier#5, if a platform is referenced by an alias (assuming that can be done) it won't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not stale Issues or PRs that are inactive but not considered stale P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants