-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Roll Abseil to Chromium's 5b92b04a2e (based on Abseil commit fc4481e968) #177059
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
Conversation
This also moves the Abseil sources from //flutter/third_party to //third_party to allow use of Chromium's GN scripts without modification. See internal issue b/400498305
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request rolls the Abseil dependency to a newer version and relocates its source code. The changes primarily consist of updating include paths and build configurations. I've identified several instances where C++ header include orders violate the Google C++ Style Guide, likely due to an automated formatting pass. Additionally, a newly added Python script uses a deprecated module, which I've suggested updating to a more modern alternative.
| import optparse | ||
| import os | ||
| import re | ||
| import shlex | ||
|
|
||
|
|
||
| class Options: | ||
| def __init__(self, output, rulename, header_guard, flags): | ||
| self.output = output | ||
| self.rulename = rulename | ||
| self.header_guard = header_guard | ||
| self.flags = flags | ||
|
|
||
|
|
||
| def GetOptions(): | ||
| parser = optparse.OptionParser() | ||
| parser.add_option('--output', help="Output header name inside --gen-dir.") | ||
| parser.add_option('--rulename', | ||
| help="Helpful name of build rule for including in the " + | ||
| "comment at the top of the file.") | ||
| parser.add_option('--gen-dir', | ||
| help="Path to root of generated file directory tree.") | ||
| parser.add_option('--definitions', | ||
| help="Name of the response file containing the flags.") | ||
| cmdline_options, cmdline_flags = parser.parse_args() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The optparse module is deprecated since Python 3.2 and is no longer developed. It's recommended to use the argparse module instead, which is the current standard for command-line parsing in Python.1
import argparse
import os
import re
import shlex
class Options:
def __init__(self, output, rulename, header_guard, flags):
self.output = output
self.rulename = rulename
self.header_guard = header_guard
self.flags = flags
def GetOptions():
parser = argparse.ArgumentParser()
parser.add_argument('--output', help="Output header name inside --gen-dir.")
parser.add_argument('--rulename',
help="Helpful name of build rule for including in the " +
"comment at the top of the file.")
parser.add_argument('--gen-dir',
help="Path to root of generated file directory tree.")
parser.add_argument('--definitions',
help="Name of the response file containing the flags.")
cmdline_options = parser.parse_args()Style Guide References
Footnotes
-
Python code should follow the Google Python Style Guide. While not a strict violation, using deprecated modules goes against best practices for maintainable code. ↩
| #include "impeller/entity/entity.h" | ||
| #include "impeller/geometry/scalar.h" | ||
| #include "impeller/geometry/sigma.h" | ||
| #include "third_party/abseil-cpp/absl/container/flat_hash_map.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/...) should be included before project-specific headers (like impeller/...).1
| #include "impeller/entity/entity.h" | |
| #include "impeller/geometry/scalar.h" | |
| #include "impeller/geometry/sigma.h" | |
| #include "third_party/abseil-cpp/absl/container/flat_hash_map.h" | |
| #include "third_party/abseil-cpp/absl/container/flat_hash_map.h" | |
| #include "impeller/entity/entity.h" | |
| #include "impeller/geometry/scalar.h" | |
| #include "impeller/geometry/sigma.h" |
Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
| #include "fml/closure.h" | ||
| #include "impeller/display_list/aiks_context.h" | ||
| #include "impeller/display_list/dl_dispatcher.h" | ||
| #include "impeller/display_list/dl_image_impeller.h" | ||
| #include "impeller/typographer/backends/skia/typographer_context_skia.h" | ||
| #include "impeller/typographer/typographer_context.h" | ||
| #include "third_party/abseil-cpp/absl/base/no_destructor.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/...) should be included before project-specific headers (like fml/... and impeller/...).1
| #include "fml/closure.h" | |
| #include "impeller/display_list/aiks_context.h" | |
| #include "impeller/display_list/dl_dispatcher.h" | |
| #include "impeller/display_list/dl_image_impeller.h" | |
| #include "impeller/typographer/backends/skia/typographer_context_skia.h" | |
| #include "impeller/typographer/typographer_context.h" | |
| #include "third_party/abseil-cpp/absl/base/no_destructor.h" | |
| #include "third_party/abseil-cpp/absl/base/no_destructor.h" | |
| #include "fml/closure.h" | |
| #include "impeller/display_list/aiks_context.h" | |
| #include "impeller/display_list/dl_dispatcher.h" | |
| #include "impeller/display_list/dl_image_impeller.h" | |
| #include "impeller/typographer/backends/skia/typographer_context_skia.h" | |
| #include "impeller/typographer/typographer_context.h" |
Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
| #include "impeller/core/shader_types.h" | ||
| #include "impeller/renderer/backend/gles/device_buffer_gles.h" | ||
| #include "impeller/renderer/backend/gles/gles.h" | ||
| #include "impeller/renderer/backend/gles/proc_table_gles.h" | ||
| #include "impeller/renderer/command.h" | ||
| #include "third_party/abseil-cpp/absl/container/flat_hash_map.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/...) should be included before project-specific headers (like impeller/...).1
| #include "impeller/core/shader_types.h" | |
| #include "impeller/renderer/backend/gles/device_buffer_gles.h" | |
| #include "impeller/renderer/backend/gles/gles.h" | |
| #include "impeller/renderer/backend/gles/proc_table_gles.h" | |
| #include "impeller/renderer/command.h" | |
| #include "third_party/abseil-cpp/absl/container/flat_hash_map.h" | |
| #include "third_party/abseil-cpp/absl/container/flat_hash_map.h" | |
| #include "impeller/core/shader_types.h" | |
| #include "impeller/renderer/backend/gles/device_buffer_gles.h" | |
| #include "impeller/renderer/backend/gles/gles.h" | |
| #include "impeller/renderer/backend/gles/proc_table_gles.h" | |
| #include "impeller/renderer/command.h" |
Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
| #include "fml/closure.h" | ||
| #include "impeller/base/thread.h" | ||
| #include "impeller/renderer/backend/gles/handle_gles.h" | ||
| #include "impeller/renderer/backend/gles/proc_table_gles.h" | ||
| #include "third_party/abseil-cpp/absl/container/flat_hash_map.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/...) should be included before project-specific headers (like fml/... and impeller/...).1
#include "third_party/abseil-cpp/absl/container/flat_hash_map.h"
#include "fml/closure.h"
#include "impeller/base/thread.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
#include "impeller/renderer/backend/gles/proc_table_gles.h"Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
| #include "flutter/third_party/re2/re2/re2.h" | ||
| #include "flutter/tools/licenses_cpp/src/comments.h" | ||
| #include "flutter/tools/licenses_cpp/src/data.h" | ||
| #include "flutter/tools/licenses_cpp/src/deps_parser.h" | ||
| #include "flutter/tools/licenses_cpp/src/filter.h" | ||
| #include "flutter/tools/licenses_cpp/src/mmap_file.h" | ||
| #include "third_party/abseil-cpp/absl/container/btree_map.h" | ||
| #include "third_party/abseil-cpp/absl/container/flat_hash_set.h" | ||
| #include "third_party/abseil-cpp/absl/log/log.h" | ||
| #include "third_party/abseil-cpp/absl/log/vlog_is_on.h" | ||
| #include "third_party/abseil-cpp/absl/status/statusor.h" | ||
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/... and flutter/third_party/...) should be included before project-specific headers (like flutter/tools/...).1
| #include "flutter/third_party/re2/re2/re2.h" | |
| #include "flutter/tools/licenses_cpp/src/comments.h" | |
| #include "flutter/tools/licenses_cpp/src/data.h" | |
| #include "flutter/tools/licenses_cpp/src/deps_parser.h" | |
| #include "flutter/tools/licenses_cpp/src/filter.h" | |
| #include "flutter/tools/licenses_cpp/src/mmap_file.h" | |
| #include "third_party/abseil-cpp/absl/container/btree_map.h" | |
| #include "third_party/abseil-cpp/absl/container/flat_hash_set.h" | |
| #include "third_party/abseil-cpp/absl/log/log.h" | |
| #include "third_party/abseil-cpp/absl/log/vlog_is_on.h" | |
| #include "third_party/abseil-cpp/absl/status/statusor.h" | |
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" | |
| #include "third_party/abseil-cpp/absl/container/btree_map.h" | |
| #include "third_party/abseil-cpp/absl/container/flat_hash_set.h" | |
| #include "third_party/abseil-cpp/absl/log/log.h" | |
| #include "third_party/abseil-cpp/absl/log/vlog_is_on.h" | |
| #include "third_party/abseil-cpp/absl/status/statusor.h" | |
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" | |
| #include "flutter/third_party/re2/re2/re2.h" | |
| #include "flutter/tools/licenses_cpp/src/comments.h" | |
| #include "flutter/tools/licenses_cpp/src/data.h" | |
| #include "flutter/tools/licenses_cpp/src/deps_parser.h" | |
| #include "flutter/tools/licenses_cpp/src/filter.h" | |
| #include "flutter/tools/licenses_cpp/src/mmap_file.h" |
Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
| #include <vector> | ||
| #include "flutter/third_party/abseil-cpp/absl/status/status.h" | ||
| #include "flutter/tools/licenses_cpp/src/data.h" | ||
| #include "third_party/abseil-cpp/absl/status/status.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/...) should be included before project-specific headers (like flutter/tools/...).1
| #include "flutter/tools/licenses_cpp/src/data.h" | |
| #include "third_party/abseil-cpp/absl/status/status.h" | |
| #include "third_party/abseil-cpp/absl/status/status.h" | |
| #include "flutter/tools/licenses_cpp/src/data.h" |
Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
| #include "flutter/third_party/re2/re2/re2.h" | ||
| #include "flutter/tools/licenses_cpp/src/license_checker.h" | ||
| #include "gtest/gtest.h" | ||
| #include "third_party/abseil-cpp/absl/status/statusor.h" | ||
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/..., gtest/..., and flutter/third_party/...) should be included before project-specific headers (like flutter/tools/...).1
| #include "flutter/third_party/re2/re2/re2.h" | |
| #include "flutter/tools/licenses_cpp/src/license_checker.h" | |
| #include "gtest/gtest.h" | |
| #include "third_party/abseil-cpp/absl/status/statusor.h" | |
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" | |
| #include "gtest/gtest.h" | |
| #include "third_party/abseil-cpp/absl/status/statusor.h" | |
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" | |
| #include "flutter/third_party/re2/re2/re2.h" | |
| #include "flutter/tools/licenses_cpp/src/license_checker.h" |
Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
| #include "flutter/third_party/re2/re2/re2.h" | ||
| #include "flutter/tools/licenses_cpp/src/mmap_file.h" | ||
| #include "third_party/abseil-cpp/absl/container/flat_hash_set.h" | ||
| #include "third_party/abseil-cpp/absl/log/check.h" | ||
| #include "third_party/abseil-cpp/absl/log/globals.h" | ||
| #include "third_party/abseil-cpp/absl/log/initialize.h" | ||
| #include "third_party/abseil-cpp/absl/log/log.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/... and flutter/third_party/...) should be included before project-specific headers (like flutter/tools/...).1
| #include "flutter/third_party/re2/re2/re2.h" | |
| #include "flutter/tools/licenses_cpp/src/mmap_file.h" | |
| #include "third_party/abseil-cpp/absl/container/flat_hash_set.h" | |
| #include "third_party/abseil-cpp/absl/log/check.h" | |
| #include "third_party/abseil-cpp/absl/log/globals.h" | |
| #include "third_party/abseil-cpp/absl/log/initialize.h" | |
| #include "third_party/abseil-cpp/absl/log/log.h" | |
| #include "third_party/abseil-cpp/absl/container/flat_hash_set.h" | |
| #include "third_party/abseil-cpp/absl/log/check.h" | |
| #include "third_party/abseil-cpp/absl/log/globals.h" | |
| #include "third_party/abseil-cpp/absl/log/initialize.h" | |
| #include "third_party/abseil-cpp/absl/log/log.h" | |
| #include "flutter/third_party/re2/re2/re2.h" | |
| #include "flutter/tools/licenses_cpp/src/mmap_file.h" |
Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
| #include "third_party/abseil-cpp/absl/flags/usage.h" | ||
| #include "third_party/abseil-cpp/absl/log/globals.h" | ||
| #include "third_party/abseil-cpp/absl/log/initialize.h" | ||
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include order appears to be incorrect. According to the Google C++ Style Guide, third-party library headers (like third_party/abseil-cpp/...) should be included before project-specific headers (like flutter/tools/...).1
| #include "flutter/tools/licenses_cpp/src/license_checker.h" | |
| #include "third_party/abseil-cpp/absl/flags/flag.h" | |
| #include "third_party/abseil-cpp/absl/flags/parse.h" | |
| #include "third_party/abseil-cpp/absl/flags/usage.h" | |
| #include "third_party/abseil-cpp/absl/log/globals.h" | |
| #include "third_party/abseil-cpp/absl/log/initialize.h" | |
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" | |
| #include "third_party/abseil-cpp/absl/flags/flag.h" | |
| #include "third_party/abseil-cpp/absl/flags/parse.h" | |
| #include "third_party/abseil-cpp/absl/flags/usage.h" | |
| #include "third_party/abseil-cpp/absl/log/globals.h" | |
| #include "third_party/abseil-cpp/absl/log/initialize.h" | |
| #include "third_party/abseil-cpp/absl/strings/str_cat.h" | |
| #include "flutter/tools/licenses_cpp/src/license_checker.h" |
Style Guide References
Footnotes
-
C++ code should follow the Google C++ Style Guide, which specifies a standard order for include statements to improve readability and avoid issues. ↩
Previously the license script looked for licenses within the engine/src/flutter tree. This PR updates the script to support the move of Abseil and the Fuchsia SDK from engine/src/flutter to engine/src/third_party. See flutter#177059 and flutter#177118
Previously the license script looked for licenses within the engine/src/flutter tree. This PR updates the script to support the move of Abseil and the Fuchsia SDK from engine/src/flutter to engine/src/third_party. See flutter#177059 and flutter#177118
reidbaker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM
Roll Flutter from 75004a639ae4 to cb18290fa45e (48 revisions) flutter/flutter@75004a6...cb18290 2025-10-24 [email protected] Remove unnecessary `deprecated` withOpacity in `text_button.0.dart` in examples (flutter/flutter#177374) 2025-10-24 [email protected] Roll Packages from 9ec29b6 to 53d6138 (3 revisions) (flutter/flutter#177502) 2025-10-24 [email protected] Roll Dart SDK from d3248b00f545 to a0480f399f8f (1 revision) (flutter/flutter#177498) 2025-10-24 [email protected] Roll Skia from a47931d09585 to 3ed332b77bec (3 revisions) (flutter/flutter#177487) 2025-10-24 [email protected] Roll Abseil to Chromium's 5b92b04a2e (based on Abseil commit fc4481e968) (flutter/flutter#177059) 2025-10-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Enhance PR template with changelog and impact details (#177333)" (#177468)" (flutter/flutter#177499) 2025-10-24 [email protected] Document DropdownMenu showTrailingIcon and decorationBuilder interaction (flutter/flutter#177488) 2025-10-24 [email protected] Roll Dart SDK from 4a65fb890852 to d3248b00f545 (1 revision) (flutter/flutter#177486) 2025-10-24 [email protected] Fix bottom sheet Semantics route label for mismatched platforms (flutter/flutter#177094) 2025-10-24 [email protected] Roll Skia from eba11de00d5b to a47931d09585 (5 revisions) (flutter/flutter#177481) 2025-10-24 [email protected] Fix Dialog Semantics label and flags for mismatched platforms (flutter/flutter#176781) 2025-10-24 [email protected] Fix drawer Semantics for mismatched platforms (flutter/flutter#177095) 2025-10-24 [email protected] Roll Dart SDK from f7751ccea102 to 4a65fb890852 (2 revisions) (flutter/flutter#177480) 2025-10-23 [email protected] Change the root path of the license crawl to engine/src (flutter/flutter#177352) 2025-10-23 [email protected] [Material] Change default mouse cursor of buttons to basic arrow instead of click (except on web) (flutter/flutter#171796) 2025-10-23 [email protected] [Desktop] Propagate SemanticsNode::identifier to AXPlatformNodeDelegate::AuthorUniqueId (flutter/flutter#175405) 2025-10-23 [email protected] Roll Skia from 59ef57f4104e to eba11de00d5b (5 revisions) (flutter/flutter#177461) 2025-10-23 [email protected] Roll customer tests (flutter/flutter#177409) 2025-10-23 [email protected] Update CHANGELOG 3.35.7 notes (flutter/flutter#177463) 2025-10-23 [email protected] Marks Mac module_uiscene_test_ios to be unflaky (flutter/flutter#177378) 2025-10-23 [email protected] Allow empty dart defines in `flutter assemble` (flutter/flutter#177198) 2025-10-23 [email protected] Roll Packages from d113bbc to 9ec29b6 (12 revisions) (flutter/flutter#177455) 2025-10-23 [email protected] Implements engine-side declarative pointer event handling for semantics. (flutter/flutter#176974) 2025-10-23 [email protected] Fix the platform name of the windowing_test target for macOS in ci.yaml (flutter/flutter#177472) 2025-10-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Enhance PR template with changelog and impact details (#177333)" (flutter/flutter#177468) 2025-10-23 [email protected] Change Flutter APIs to use spans (flutter/flutter#177272) 2025-10-23 [email protected] Roll Dart SDK from 020988602772 to f7751ccea102 (4 revisions) (flutter/flutter#177449) 2025-10-23 [email protected] [macOS] Implement regular window (flutter/flutter#176361) 2025-10-23 [email protected] Enhance PR template with changelog and impact details (flutter/flutter#177333) 2025-10-23 [email protected] Roll Skia from 6e6acf6644ef to 59ef57f4104e (2 revisions) (flutter/flutter#177436) 2025-10-23 [email protected] Add directional static members to AlignmentGeometry. (flutter/flutter#176571) 2025-10-23 [email protected] Roll ICU from 1b2e3e8a421e to ff35c4f9df23 (5 revisions) (flutter/flutter#177434) 2025-10-23 [email protected] Adds a new CI build for Linux host DDM-enabled artifacts (flutter/flutter#177252) 2025-10-23 [email protected] Roll Skia from 920cdcadd74c to 6e6acf6644ef (1 revision) (flutter/flutter#177432) 2025-10-23 [email protected] Roll Skia from 8cd449e4953b to 920cdcadd74c (6 revisions) (flutter/flutter#177426) 2025-10-23 [email protected] Added ahem license (flutter/flutter#177423) 2025-10-23 [email protected] Roll Dart SDK from 75f6ccb9bdc5 to 020988602772 (1 revision) (flutter/flutter#177421) 2025-10-23 [email protected] [web] Set `pointer-events: none` for img-element-backed images (flutter/flutter#177418) 2025-10-22 [email protected] Remove the x64 version of build_ios_framework_module_test (flutter/flutter#177136) 2025-10-22 [email protected] Fix accessibility events not being correctly translated to ATK (flutter/flutter#176991) 2025-10-22 [email protected] Roll Skia from b55bd60ed95b to 8cd449e4953b (8 revisions) (flutter/flutter#177405) 2025-10-22 [email protected] Roll Dart SDK from c23010c4f9e6 to 75f6ccb9bdc5 (4 revisions) (flutter/flutter#177399) 2025-10-22 [email protected] Fixes crash when adding and removing mulitple page-based route (flutter/flutter#177338) 2025-10-22 [email protected] Move child parameter to end of RefreshIndicator constructor (flutter/flutter#177019) 2025-10-22 [email protected] refactor: migrate OpenUpwardsPageTransitionsBuilder to widgets (flutter/flutter#177080) 2025-10-22 [email protected] Delete stray 'text' file (flutter/flutter#177355) ...
Previously the license script looked for licenses within the engine/src/flutter tree. This PR updates the script to support the move of Abseil and the Fuchsia SDK from engine/src/flutter to engine/src/third_party. See flutter#177059 and flutter#177118
…68) (flutter#177059) This also moves the Abseil sources from //flutter/third_party to //third_party to allow use of Chromium's GN scripts without modification. See internal issue b/400498305
This also moves the Abseil sources from //flutter/third_party to //third_party to allow use of Chromium's GN scripts without modification.
See internal issue b/400498305