Generated .isolate files for Android Swarming Contain Unused Files |
|||||
Issue descriptiongn gen --args='target_os="android"' out/Debug tools/mb/mb.py isolate //out/Debug android_webview_unittests cat out/Debug/android_webview_unittests.isolate Files that I think are not needed: *.mojom.js *lint/config.xml *lint/result.xml *.so Files that may or may not be: pyproto/* (included via //third_party/protobuf:py_proto. Needed for host-driven tests?)
,
Mar 11 2016
*lint/ files are only used at build time and should not be necessary. re *.so files: exactly, they're in the apk.
,
Mar 11 2016
Given that these are used largely for test binaries, it's possible this may be more trouble than it's worth to fix the .so problem ...
,
Mar 11 2016
How do we get around this in the GYP/static isolates? Are the .so's excluded in Android component builds explicitly or something?
,
Mar 11 2016
Another thought here:
Since the files to push to device are a strict subset of the files to push to swarming, it might make sense to have:
unittest_apk() {
data = files_to_push_to_device
}
generate_test_script() {
data = files_required_by_test_runner
data_deps = unittest_apk
}
This was we can generate both .isolate files via gn desc runtime_deps.
This is assuming we can get the data of unittest_apk to not include extra things.
,
Mar 11 2016
In thinking about this more (given thakis's questions on the //base change in https://codereview.chromium.org/1784273003/ , and bug 589318 ), I realized we have another problem. In order to push the files to the device, we need the list of files in files_to_push_to_device . Currently we get that from the checked-in isolate. If we get rid of the checked-in isolate, we have to recreate the file. Currently the way we create the files for swarming is to generate them *after* we run the build. However, in order for the list of files to be included in the (swarming) isolate, it needs to be listed as a data dependency *in* the build, which isn't currently possible. In addition, we need files_to_push_to_device to *always* exist in a build, else you can't run the tests even in the non-swarming case. I talked to brettw@ about this briefly. We think we probably end up needing a dump_runtime_deps_to_file() built-in function in order to handle this, and then we can update the test scripts to take a list of files rather than parse the isolate format. It's possible (although not entirely clear to me yet) that there's also a fair amount of overlap between this problem and the bundle_data()/create_bundle() code we just added for iOS. We should work out the rest of the issues with Android data deps, isolates, apks, etc. and come up with a solution for them all at once, and then we can see if we can find commonality between android, ios, and desktop data_deps such that we can refactor/simplify things again.
,
Mar 12 2016
Yet another option (which I'm now leaning towards): Maybe we should just get rid of having side-loaded files for android tests entirely, and instead package the needed files as Android assets so that they are embedded in the .apk. This would mean needing to define android_assets() targets for all of the tests, as well as updating the code to load it in the correct way. Perhaps the android_assets could be implicitly declared by test.gni, so that for non-Android the test data can be declared as data, for android, declared as android_assets, and for ios declared as bundle_data().
,
Mar 14 2016
FWIW, my plan for iOS is to have the "test" template create an application bundle on iOS. This will use the bundle_data/create_bundle. Those two targets are marketed as iOS/OS X generic, but unless there are files named *.xcassets/*.imagefile/* then I think you can reuse them for Android too to generate *.apk.
,
Mar 15 2016
After even more thought (my brain hurts), I've come to the same conclusion as in #6. Additionally, I think that for now we should just work-around the primitive not existing by running gn desc at build-time. Example: https://codereview.chromium.org/1805643002
,
Mar 22 2016
Looked into the file having so many things that are unnecessary. They fall into 3 buckets: 1. .so files that GN implicitly decides are data 2. data files (such as icudtl.dat) that are packed into the android .apk but are declared as data. These could probably be if(android)'ed so as to not appear 3. lint resultes. The lint results I'm not sure how to fix. They are declared data_deps() in order have them build in parallel to other targets. GN has no other way to declare this though (e.g. validation_deps) I have an example CL which just takes in the list of runtime_deps and filters out #1, #2, and #3: https://codereview.chromium.org/1805643002/
,
Mar 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f commit 0f8ad42ac1a6b159655c51bf7095ff2d21ae540f Author: agrieve <agrieve@chromium.org> Date: Thu Mar 31 22:00:07 2016 GN: Add write_runtime_deps variable Allows targets to use runtime_deps at build-time. BUG= 593416 Review URL: https://codereview.chromium.org/1804303004 Cr-Commit-Position: refs/heads/master@{#384411} [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/BUILD.gn [add] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/group_target_generator_unittest.cc [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/runtime_deps.cc [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/scheduler.cc [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/scheduler.h [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/target.cc [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/target.h [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/target_generator.cc [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/target_generator.h [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/target_unittest.cc [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/variables.cc [modify] https://crrev.com/0f8ad42ac1a6b159655c51bf7095ff2d21ae540f/tools/gn/variables.h
,
Apr 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9b7e031dd50a0381dc25d5c487b8631f886bf079 commit 9b7e031dd50a0381dc25d5c487b8631f886bf079 Author: agrieve <agrieve@chromium.org> Date: Tue Apr 12 14:22:49 2016 Stop including android lint results in runtime_deps BUG= 593416 Review URL: https://codereview.chromium.org/1876823003 Cr-Commit-Position: refs/heads/master@{#386679} [modify] https://crrev.com/9b7e031dd50a0381dc25d5c487b8631f886bf079/build/config/android/internal_rules.gni
,
Apr 13 2016
,
Apr 15 2016
Figured out that is_asan=true adds more unwanted things: ../../tools/valgrind/asan/ ../../third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer
,
Apr 15 2016
And also some more: pyproto/google/__init__.py pyproto/google/protobuf/__init__.py pyproto/google/protobuf/descriptor.py pyproto/google/protobuf/descriptor_database.py pyproto/google/protobuf/descriptor_pool.py pyproto/google/protobuf/message.py pyproto/google/protobuf/message_factory.py pyproto/google/protobuf/reflection.py pyproto/google/protobuf/service.py pyproto/google/protobuf/service_reflection.py pyproto/google/protobuf/text_format.py pyproto/google/protobuf/descriptor_pb2.py pyproto/google/protobuf/internal/__init__.py pyproto/google/protobuf/internal/api_implementation.py pyproto/google/protobuf/internal/containers.py pyproto/google/protobuf/internal/cpp_message.py pyproto/google/protobuf/internal/decoder.py pyproto/google/protobuf/internal/encoder.py pyproto/google/protobuf/internal/enum_type_wrapper.py pyproto/google/protobuf/internal/generator_test.py pyproto/google/protobuf/internal/message_listener.py pyproto/google/protobuf/internal/python_message.py pyproto/google/protobuf/internal/type_checkers.py pyproto/google/protobuf/internal/wire_format.py
,
Apr 20 2016
While we should fix this, I don't think this blocks the GN migration.
,
Apr 20 2016
The pyproto files are due to net_testsupport. There is also now: Mojo Applications/catalog/manifest.json, which is being pulled in through //content:resources_grit
,
Apr 20 2016
The pyproto files are due to net_testsupport. There is also now: Mojo Applications/catalog/manifest.json, which is being pulled in through //services/catalog:lib source_set
,
Apr 29 2016
This is probably "fixed enough". The generator script has some blacklists for removing most unwanted files. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dpranke@chromium.org
, Mar 11 2016Labels: Build-Tools-GN