Commit 5b58507
committed
Consolidate cmake error targets to match BUCK fboss-error
The BUCK build has a single `fboss-error` target containing:
- `FbossError.h`
- `FbossHwUpdateError.h`
- `SysError.h`
The cmake build previously had two separate targets:
- `error` (only `FbossError.h`)
- `fboss_error` (`FbossError.h` and `SysError.h`)
This caused divergence between the two build systems.
Changes made:
1. Remove the `error` target from `cmake/Agent.cmake`
2. Update `fboss_error` to include all three headers to match BUCK:
- `FbossError.h`
- `FbossHwUpdateError.h` (was missing)
- `SysError.h`
3. Update `fboss_error` dependencies to match BUCK exactly:
- `fboss_cpp2` (matches `//fboss/agent/if:fboss-cpp2-types`)
- `Folly::folly` (matches `//folly:conv` and `//folly/logging:logging`)
- Remove `fboss_types` (not in BUCK dependencies)
- Plot twist: add`switch_config_cpp2` dependency to `logging_util` target.
This was previously provided transitively via `fboss_error` -> `fboss_types`
-> `switch_config_cpp2`, but removing `fboss_types` from `fboss_error`
broke that chain. The `logging_util` target directly includes
`switch_config_types.h` so it needs to declare this dependency explicitly.
4. Replace all 23 instances of `error` dependency with `fboss_error`
across 16 cmake files
5. Add missing `fboss_error` dependency to `external_phy` target
(required in the BUCK file but was missing in cmake)
Additionally, two more fixes had to be fixed to address all the
remaining issue with `system_scale_test_utils` found by the script,
and those were resolved once again by matching BUCK files:
- Remove bundled source files (`PortFlapHelper.cpp`, `MacLearningFloodHelper.cpp`)
- Add dependencies on existing `port_flap_helper` and `mac_learning_flood_helper`
targets (defined in `AgentTestUtils.cmake`)
Oh and a few more errors that sneaked into the code more recently and
that I missed in facebook#833 because I was working off a fork a few days old:
1. In `cmake/AgentState.cmake`, the `state` library was missing a dependency
on `agent_features` (from Buck dep: `//fboss/agent:agent_features`)
2. In `cmake/fsdb/FsdbIf.cmake`, the `thriftpath_lib` library was missing a
dependency on `fsdb_utils` (from Buck dep: `//fboss/fsdb/common:utils`)
3. In `cmake/fsdb/FsdbTests.cmake`, the `thriftpath_test_cpp2` library was
missing a dependency on `common_cpp2` (from Buck dep: `//fboss/agent/if:common`)
Bonus: add duplicate cmake target detection to `buck_cmake_dep_checker.py`,
because while fixing that last error I inadvertently made the mistake of
creating a duplicate rule in a cmake file that was defined in another.
Also fix the tool to handle thrift sub-targets. Buck thrift_library generates
sub-targets like `//path:name-cpp2-types`, `//path:name-cpp2-services`, etc.
The tool now maps these to the corresponding cmake thrift target, which
uncovered 9 additional pre-existing missing dependencies that are now fixed:
- Add `switch_config_cpp2` to `logging_util` (includes `switch_config_types.h`)
- Add `ctrl_cpp2` to `label_forwarding_action` (includes `ctrl_types.h`)
- Add `agent_hw_test_ctrl_cpp2` to `aqm_test_utils`
- Add `data_corral_service_cpp2`, `fan_service_cpp2`, `rackmon_cpp2` to `fboss2_lib`
- Add `fboss_common_cpp2` to `product_info`
- Add `fan_service_cpp2` to `cross_config_validator`
- Add `fan_service_cpp2` to `fan_service_config_validator`
- Add `transceiver_cpp2` to `qsfp_stats`
Now the script doesn't report any errors anymore and could potentially
be used in CI to ensure no further differences between the BUCK build
and the cmake build are introduced.1 parent ccd34a4 commit 5b58507
File tree
26 files changed
+87
-41
lines changed- cmake
- fsdb
- fboss/oss/scripts
26 files changed
+87
-41
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
148 | 148 | | |
149 | 149 | | |
150 | 150 | | |
151 | | - | |
| 151 | + | |
152 | 152 | | |
153 | 153 | | |
154 | 154 | | |
| |||
260 | 260 | | |
261 | 261 | | |
262 | 262 | | |
263 | | - | |
| 263 | + | |
264 | 264 | | |
265 | 265 | | |
266 | 266 | | |
| |||
432 | 432 | | |
433 | 433 | | |
434 | 434 | | |
435 | | - | |
436 | | - | |
437 | | - | |
438 | | - | |
439 | | - | |
440 | | - | |
441 | | - | |
442 | | - | |
443 | | - | |
444 | 435 | | |
445 | 436 | | |
446 | 437 | | |
| |||
491 | 482 | | |
492 | 483 | | |
493 | 484 | | |
| 485 | + | |
494 | 486 | | |
495 | 487 | | |
496 | 488 | | |
497 | 489 | | |
498 | 490 | | |
499 | | - | |
500 | 491 | | |
501 | 492 | | |
502 | 493 | | |
| |||
514 | 505 | | |
515 | 506 | | |
516 | 507 | | |
517 | | - | |
| 508 | + | |
518 | 509 | | |
519 | 510 | | |
520 | 511 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
84 | 84 | | |
85 | 85 | | |
86 | 86 | | |
87 | | - | |
| 87 | + | |
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| 21 | + | |
21 | 22 | | |
22 | 23 | | |
23 | 24 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
| 49 | + | |
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
47 | | - | |
| 47 | + | |
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
| 26 | + | |
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
118 | 118 | | |
119 | 119 | | |
120 | 120 | | |
121 | | - | |
| 121 | + | |
| 122 | + | |
122 | 123 | | |
123 | 124 | | |
124 | 125 | | |
| |||
143 | 144 | | |
144 | 145 | | |
145 | 146 | | |
146 | | - | |
| 147 | + | |
147 | 148 | | |
148 | 149 | | |
149 | 150 | | |
| |||
156 | 157 | | |
157 | 158 | | |
158 | 159 | | |
159 | | - | |
| 160 | + | |
| 161 | + | |
160 | 162 | | |
161 | 163 | | |
162 | 164 | | |
0 commit comments