migrate dirs whose runtime.python_binary kwargs are fbcode-only#21388
migrate dirs whose runtime.python_binary kwargs are fbcode-only#21388bigfootjon wants to merge 2 commits into
Conversation
…rules)
Summary:
Chunk 8 of fbcode/executorch TARGETS->BUCK migration. 3 directories that
were skipped from chunk 6 because they already had a targets.bzl, but
their TARGETS file still contained extra fbcode-only rules.
- devtools/etdump/fb/tests: TARGETS adds a python_unittest serialize_test
on top of the cxx_test in targets.bzl. Merged via 7B pattern.
- devtools/etdump/tests: same shape as above. Merged via 7B pattern.
- exir/fb: targets.bzl previously had only module-level constants
(executorch_portable_kernel_lib, executorch_aten_mode_lib) and no
define_common_targets function. Added a new define_common_targets
that early-returns when not is_fbcode and otherwise defines the
bento_deps_* python_library rules from the old TARGETS.
For each, the BUCK file now calls
`define_common_targets(is_fbcode = is_fbcode())` via fbsource_utils.
Differential Revision: D109082055
Summary:
Chunk 9 of fbcode/executorch TARGETS->BUCK migration. 4 directories that
were attempted in earlier chunks but reverted because their
`runtime.python_binary` calls use kwargs the xplat macro rejects
(`main_src`, `srcs = ["data_utils.py"]` triggering the
"default_only is not allowed to be specified" error, etc.).
Migrated using the chunk-6 gate-everything-on-is_fbcode pattern: each dir
now has a thin BUCK calling `define_common_targets(is_fbcode = is_fbcode())`
and a targets.bzl whose `define_common_targets` early-returns when not in
fbcode. This preserves the pre-migration behavior where these dirs were
fbcode-only by virtue of being TARGETS-only.
Directories migrated:
- devtools/inspector
- examples/devtools/scripts
- extension/pybindings/fb/test
- extension/training/examples/CIFAR
CIFAR's existing targets.bzl was edited to add `is_fbcode = False` param
and an early-return guard at the top of `define_common_targets`.
runtime/test was attempted but reverted: the dir's PACKAGE file calls
`ci.buckconfig("executorch.event_tracer_enabled", "true")` and that
buckconfig key is not on the xplat allowlist, so xplat fails to evaluate
the package once it has a BUCK file. Needs a separate fix to the PACKAGE
file (probably gating the ci.buckconfig call on is_fbcode) before this
dir can be migrated.
Differential Revision: D109082048
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21388
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New FailuresAs of commit 32f3576 with merge base 6cd7ecd ( NEW FAILURES - The following jobs have failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@bigfootjon has exported this pull request. If you are a Meta employee, you can view the originating Diff in D109082048. |
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
Migrates several fbcode-only directories from TARGETS to BUCK while preserving prior behavior in xplat by gating target definitions on is_fbcode().
Changes:
- Added thin
BUCKentrypoints that calldefine_common_targets(is_fbcode = is_fbcode()). - Introduced/updated
targets.bzlfiles that early-return when not in fbcode to avoid xplat-incompatibleruntime.python_binarykwargs. - Completed migration cleanup by removing
TARGETSfiles and folding fbcode-only unit tests behind anis_fbcodegate.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| extension/training/examples/CIFAR/targets.bzl | Adds is_fbcode param + early-return guard to keep CIFAR fbcode-only. |
| extension/training/examples/CIFAR/BUCK | New BUCK entrypoint calling define_common_targets with is_fbcode(). |
| examples/devtools/scripts/targets.bzl | New fbcode-gated targets definition (python binaries). |
| examples/devtools/scripts/BUCK | New BUCK entrypoint calling define_common_targets with is_fbcode(). |
| devtools/inspector/targets.bzl | Moves inspector targets into a fbcode-gated targets.bzl. |
| devtools/inspector/BUCK | New BUCK entrypoint calling define_common_targets with is_fbcode(). |
| devtools/inspector/TARGETS | Removed as part of TARGETS→BUCK migration. |
| devtools/etdump/tests/targets.bzl | Adds is_fbcode param and gates python unittest target creation on it. |
| devtools/etdump/tests/BUCK | Updates BUCK to pass is_fbcode() into define_common_targets. |
| devtools/etdump/tests/TARGETS | Removed as part of TARGETS→BUCK migration. |
Comments suppressed due to low confidence (1)
devtools/etdump/tests/targets.bzl:9
- The docstring still claims this directory has both TARGETS and BUCK callers, but TARGETS was removed as part of this migration. Updating the docstring avoids misleading future edits.
"""Defines targets that should be shared between fbcode and xplat.
The directory containing this targets.bzl file should also contain both
TARGETS and BUCK files that call this function.
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def define_common_targets(): | ||
| def define_common_targets(is_fbcode = False): | ||
| """Defines targets that should be shared between fbcode and xplat. |
Summary:
Chunk 9 of fbcode/executorch TARGETS->BUCK migration. 4 directories that
were attempted in earlier chunks but reverted because their
runtime.python_binarycalls use kwargs the xplat macro rejects(
main_src,srcs = ["data_utils.py"]triggering the"default_only is not allowed to be specified" error, etc.).
Migrated using the chunk-6 gate-everything-on-is_fbcode pattern: each dir
now has a thin BUCK calling
define_common_targets(is_fbcode = is_fbcode())and a targets.bzl whose
define_common_targetsearly-returns when not infbcode. This preserves the pre-migration behavior where these dirs were
fbcode-only by virtue of being TARGETS-only.
Directories migrated:
CIFAR's existing targets.bzl was edited to add
is_fbcode = Falseparamand an early-return guard at the top of
define_common_targets.runtime/test was attempted but reverted: the dir's PACKAGE file calls
ci.buckconfig("executorch.event_tracer_enabled", "true")and thatbuckconfig key is not on the xplat allowlist, so xplat fails to evaluate
the package once it has a BUCK file. Needs a separate fix to the PACKAGE
file (probably gating the ci.buckconfig call on is_fbcode) before this
dir can be migrated.
Differential Revision: D109082048