Commit graph

29 commits

Author SHA1 Message Date
Mark Dacek
b2441c52a4 Revert "Revert "Modify symlink_forest to rerun when soong_build has changed.""
This reverts commit aa5cc2cd6a.

Reason for revert: Fixed the bug in the original draft.
There was an error in how we compared the file's MTime to the
file's contents.

Bug: 300288299
Test: presubmits
Test: build/soong/tests/run_integration_tests.sh

Change-Id: I9e53432c0842c0b9fc13fe20d30ce9af37640c7f
2023-10-05 17:11:50 +00:00
Mark Dacek
aa5cc2cd6a Revert "Modify symlink_forest to rerun when soong_build has changed."
This reverts commit 23a4120c57.

Reason for revert: broke soong_integration

Change-Id: I4d51841756675b3745244d23e13aefda0916614b
2023-10-02 23:40:33 +00:00
MarkDacek
23a4120c57 Modify symlink_forest to rerun when soong_build has changed.
Also remove existing symlink_forest_version functionality.

This remedies issues pertaining to b/300122962 - symlinks not
clearing after a revert

Timing wise - this doesn't present a performance regression
on a clean build. When soong_build changes, it's considerably longer
but no different from the time when symlink_forest_version is changed.

Bug: 300288299
Test: build/soong/tests/symlink_forest_rerun_test.sh
Change-Id: I0e95aac315dfea7ea3b8bb9a3eb0c6408300bd3b
2023-09-29 22:53:07 +00:00
Cole Faust
076aa2aeeb Increment symlink_forest_version
We've had some incrementality issues with the symlink forest since
aosp/2673616, clear old symlink forests to get rid of the bad symlinks.

Bug: 300129912
Test: Presubmit
Change-Id: Ic23c980b68ebcc8b8788d56e53435f0a89d82b28
2023-09-13 08:56:54 -07:00
Sebastian Pickl
90355f79bf Revert "Change symlink_forest to use relative symlinks."
Revert submission 2673616-relativesymlinks-fix

Reason for revert: this breaks tests verified with go/abtd https://android-build.googleplex.com/builds/abtd/run/L36000000963001181

Bug: 300122962

Reverted changes: /q/submissionid:2673616-relativesymlinks-fix

Change-Id: I5a97c4fbe4df5727c0604a07137093d0f00c7776
2023-09-12 18:51:39 +00:00
MarkDacek
b98b3a429f Change symlink_forest to use relative symlinks.
Also add script to remove the Bazel output base.
This will assist with supporting movable checkouts alongside
mixed builds.

Bug: 259191764
Test: m && (move topic and prepare_moved_top.sh) && m
Test: m && prepare_moved_top.sh && m
Test: build/soong/tests/relative_symlinks_test.sh

Change-Id: I0f53da8d99f752fad496cf3ac61b01f001b7296d
2023-09-12 04:02:23 +00:00
Cole Faust
5db92090e1 Clean old symlink forests
There was an incrementality bug in the symlink forest generation that
was recently fixed. However, if the appropriate files don't get touched,
the issue will remain. Add a mechanism to clean old symlink forests
so that when we fix incrementality bugs we can ensure they don't remain
in any old out directories.

Bug: 276349152
Test: Presubmits
Change-Id: I36664dfb0ca7227e3e1f89de859ebccb808c5f15
2023-04-05 11:44:49 -07:00
Cole Faust
3f4f521711 Fix issue merging bp2build files with handcrafted ones
It was possible for the merged content to end up back in the bp2build
generated file because there was a symlink from the symlink forest to
the bp2build generated file.

Remove the symlink if it exists.

Bug: 276349152
Test: m bp2build, add a handcrafted file in the same folder as a Android.bp file, m bp2build again, check that the symlink forest version is not a symlink
Change-Id: Id64aa3addebcf0c6b1728389f21ae246796aaf8d
2023-04-05 09:08:38 -07:00
Chris Parsons
1a12d03230 Avoid rewriting soong_build outputs if unchanged
This changes bp2build codegen, symlink forest generation, and
soong_build so that they do not rewrite output files if the contents are
unchanged.

Bug: 266983462
Test: m droid
Test: canonical_perf.sh benchmarking
Test: Manually verified that rerunning analysis did not regenerate
out/soong/workspace/prebuilts/sdk/BUILD.bazel unless contents changed

Change-Id: I5ec227df7a32b53c7fa0d741fb1403a51931024b
2023-02-10 15:34:06 -05:00
Usta (Tsering) Shrestha
c4c07b12b6 reduce forest generation to be incremental
Previously, symlink forest generation involved removing the entire
symlink forest and recreating it from scratch. With this change,
a) symlinks which need not change are untouched,
b) symlinks pointing to the wrong location are fixed, and
c) symlinks which should no longer exist are removed.

On AOSP on my local machine, this reduces the symlink forest generation
step from 2.5s to 1.1s clean, and 0.6s when a single file is added to
a source directory.

Bug: 257528847
Test: m bp2build, touch `fakefile` under the forest, remove a file
from the source tree, rerun m bp2build. Manually verify the new forest
does not retain the link to the deleted source file, and that fakefile
no longer exists in the forest.

Change-Id: I481371ae487e9419af6a3a4370c552578b07d650
2023-02-07 06:23:43 +00:00
Cole Faust
358ba4f0d8 Make symlink forest errors deterministic
We were looping over a map, and thus processing files
in a non-deterministic order. Some tests check for errors
produced during the symlink forest creation and they would
flake due to this nondeterminism.

Test: m nothing
Change-Id: Ie1a4cf98e341e4a951f110b7d5611bb69c4ddd2b
2023-01-18 15:02:03 -08:00
Usta Shrestha
071f6c29a3 for consistency symlink_tree failure = os.Exit(1)
Brings down time taken to fail from 18s to 13s (on aosp branch)

Test: temporarily coded random failure
Bug: N/A
Change-Id: Ib694eec977293f4dd7054e779d1b82b8cace93f6
2023-01-13 14:07:10 -05:00
Usta Shrestha
da15c61add metrics: count symlink/mkdir in symlink forest
Bug: 260029212
Test: inspect out/bp2build_metrics.pb
Change-Id: Ia84c095a8d7b129cca629256b6c626c726cbfab1
2023-01-12 14:52:56 -05:00
Lukacs T. Berki
bc5f731791 Multithread symlink forest removal.
This is a second attempt at aosp/2273288 which got rolled back because
it turned out that:

1. We make ~120K symlinks in AOSP (!), all of which need to be deleted
2. System calls are sometimes slow
3. Golang spawns a new OS-level thread for each blocking system calls to
   keep cores busy

All this together means that we sometimes had 10K system calls in
flight, which meant 10K OS-level threads, which is when Go gives up and
says "I created too many threads, please help".

The fix is to move the system calls into a pool of goroutines, which
soon end up on a pool of threads (since they mostly do blocking system
calls)

Test: Presubmits.
Change-Id: Ia9aefff3b0ed373f09bb6c8b2ec1d8b0f00b213b
2022-10-31 16:04:13 +00:00
Christopher Parsons
ed2873aea0 Revert "Multithread symlink forest removal."
This reverts commit 6b236f1607.

Reason for revert: Breaks ab/aosp-master-bazel incremental builds. Details on b/254338319

Change-Id: I37eeeda50cff0475d91e7926fdf74216975a0037
2022-10-28 15:43:48 +00:00
Lukacs T. Berki
6b236f1607 Multithread symlink forest removal.
This makes symlink forest creation ca. 2x faster again, taking 2-3
seconds instead of 5.

Who would have thought that os.RemoveAll() is slow.

Test: Presubmits.
Change-Id: I91e41319c972dbf1113cf723e383c785433c18b9
2022-10-28 07:15:55 +00:00
Lukacs T. Berki
647e7abfa2 Build the symlink tree on multiple threads.
This makes it take ~5 seconds on AOSP instead of ~10. Frankly, the
speedup is somewhat disappointing but at least the code is not
complicated.

Test: Presubmits.
Change-Id: Icf94d7ca8bd80c458d014f4cf4cc1be7138deaa6
2022-10-27 10:17:20 +00:00
Lukacs T. Berki
c541cd27fa Create Bazel symlink forest in a separate process.
This helps with incrementality a lot: the symlink forest must depend on
almost every directory in the source tree so that if a new file is added
or removed from *anywhere*, it is regenerated.

Previously, we couldn't do this without invoking bp2build, which is
quite wasteful because bp2build takes way more time than the symlink
forest creation, even though we do the latter in a very suboptimal way
at the moment.

This means that if a source file is added or removed (which does not
affect globs), we don't pay the cost of bp2build anymore.

Also refactored symlink_forest.go on the side. Too much state was being
passed around in arguments.

This change reimplements aosp/2263423 ; the semantics of not touching an
output file is the exact same as order-only inputs and the latter is a
bit fewer lines of code.

Test: Presubmits.
Change-Id: I565c580df8a01bacf175d56747c3f50743d4a4d4
2022-10-27 08:08:45 +00:00
Usta Shrestha
783ec5c72c Remove a self-dependency of bp2build
bp2build (i.e. bootstrap.ninja#bp2build_workspace_marker) generate $OUT/soong/bp2build/**/BUILD.bazel files
Having them as a dependency would thus make bp2build_workspace_marker stale upon incremental builds (because we don't re-touch the marker file if it already exists)

See Also: https://android-review.googlesource.com/c/platform/build/soong/+/2263423

Bug: b/253450880
Test: NINJA_ARGS='-d explain' m --bazel-mode adbd
      repeat and see if bp2build is rerun
prior to this CL, one would see the input /usr/local/google/home/usta/aosp/out/soong/bp2build/build/make/tools/BUILD.bazel trigger bp2build over and over again

Change-Id: I904cd333a5d6ef506fc4019eda7623ef96a1daa3
2022-10-25 13:46:20 +00:00
Usta Shrestha
49d04e89d8 cosmetics
Test: m --bazel-mode nothing
Bug: b/239044236
Change-Id: Iaffc315c696f2fa19a2525009010d5964cf5a7d1
2022-10-21 20:09:23 +00:00
Cole Faust
fd88414b4c Fix incrementality bug with merged BUILD files
The merged build files should be added as ninja dependencies,
so we rerun when they're changed.

Fixes: 246552590
Test: m bp2build, m bp2build again and observe it didn't rerun. Then add a comment in external/protobuf/BUILD.bazel, run m bp2build again, and observe it reruns
Change-Id: I26ed035cc0a894500a192f9aa3371fb46519689b
2022-10-07 13:11:45 -07:00
Cole Faust
324a92e404 Always merge build files
Previous behavior:

- Packge not listed in bp2buildKeepExistingBuildFile:
    - Use bp2build generated build file
- Package listed in bp2buildKeepExistingBuildFile:
    - Use handcrafted build file even if there were allowlisted bp2build
      modules in the same package.
- Package listed in bp2buildKeepExistingBuildFile and a soong module has
  a bp2build: { label } attribute:
    - Merge the handcrafted and bp2build generated build files

New behavior:

- Packge not listed in bp2buildKeepExistingBuildFile:
    - Use bp2build generated build file
- Package listed in bp2buildKeepExistingBuildFile:
    - Merge with bp2build generated build file.

Bug: 234167862
Test: ./build/bazel/ci/bp2build.sh
Change-Id: Ifbaf4f8f0f5158b5b2bd6d534eb2311e2e5f399b
2022-08-30 16:07:23 -07:00
Usta Shrestha
db46a9bbb8 Cosmetic Changes plus log errors
Test: N/A
Bug: N/A
Change-Id: I39e622a93270c922bdbea27ca79632a606431a2c
2022-08-02 20:24:57 +00:00
Sasha Smundak
0fd93e0756 Output informational messages only when BP2BUILD_VERBOSE is set.
Test: manual
Change-Id: Ieeb00a002e07b04449d70614ce205c47c1dd7bce
2022-07-18 13:34:44 -07:00
Jingwen Chen
d4b1dc8b2a bp2build: add support for planting unresolved symlinks in the symlink forest.
In the isDir check, there's a possibility that the Stat check fails
because the path is an unresolved symlink. Verify it with Lstat (which
would succeed, since it doesn't follow links), and treat it like a file
if Lstat succeeds.

Test: new integration test
Fixes: 232370097
Change-Id: I9807ca363a5dbdc20639b489b54627bd2cc1ca60
2022-05-17 12:17:53 +00:00
Lukacs T. Berki
e3487c8848 Add a test for correctness of C++ compilation.
This required the following:

- Adding Platform_base_sdk_extension_version to default soong.variables
- Teaching the symlink tree creation code to understand symlinks
- Making finder.go follow symlinks when requested

Adding yet another knob is unfortunate, but I can't allow that
unconditionally because the Android code base contains a number of
symlinks giving rise to infinite directory trees because they point back
to their parent and this seemed preferable to adding complicated logic
like "follow symlink but if only its fully resolved version does not
point under the source tree".

I could be convinced about the latter, though.

Test: Presubmits.
Change-Id: I453f6b7e5334771f5832c700db00f9d24ed1d82f
2022-05-04 09:12:01 +02:00
Lukacs T. Berki
b21166e236 Make symlink_forest.go prefer generated files.
Now, if the same file exists in the generated tree and the source tree,
it symlinks in the generated file instead of failing outright.

Drive-by fix: print errors for all conflicts instead of bailing out on
the first one.

Test: Presubmits (including the two new tests)
Change-Id: Ifb5b3fc89b5454d231966bfa4e61c22cd69834f3
2021-04-21 12:24:27 +02:00
Lukacs T. Berki
3f9416ea80 Rename cryptic symbols in symlink_forest.go .
Test: Presubmits.
Change-Id: Ib41e5ba001924d6fca72410bc0895747d6e55d37
2021-04-20 13:08:11 +02:00
Lukacs T. Berki
b353cca496 Create a synthetic Bazel workspace.
It's under out/soong/workspace and is a symlink forest that merges BUILD
files generated by bp2build and the source tree.

bazel-* symlinks and $OUTDIR are currently ignored.

Test: Presubmits.
Change-Id: If14df23bd0f6a6f2d7d1140f95213f2aee66d652
2021-04-20 10:00:02 +02:00