Skip to content

Auto populate the namespace list to rename in desktop packaging #1000

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

Merged
merged 43 commits into from
Jul 27, 2022
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
6a78ebc
Change desktop packaging to automatically get the list of C++ namespa…
jonsimantov Jun 17, 2022
d3a057c
Add additional namespace to ignore.
jonsimantov Jun 17, 2022
67bc714
Add readme note
jonsimantov Jun 17, 2022
0423843
After packaging, scan the libraries to ensure no leaking namespaces.
jonsimantov Jun 17, 2022
8ae3143
Update to newer demumble version
jonsimantov Jun 17, 2022
0d9065f
Fix hash
jonsimantov Jun 17, 2022
61e802f
demumble now requires python3
jonsimantov Jun 17, 2022
baa7ce7
Add missing parallel install
jonsimantov Jun 18, 2022
69ffbdd
Merge branch 'main' into bugfix/packaging_auto_hide_namespaces
jonsimantov Jun 23, 2022
4e340bd
Fix a bug in merge_libraries that did not rename all symbols.
jonsimantov Jun 23, 2022
5e33487
Turn on debug output
jonsimantov Jun 23, 2022
c1b0c6f
Remove top-level firestore namespace from ignore list.
jonsimantov Jun 23, 2022
8b17dbf
Don't ignore firestore namespace when checking for invalid ones
jonsimantov Jun 23, 2022
a6c90d0
Fix a bug that failed to find namespaces with tildes.
jonsimantov Jun 23, 2022
d01a21e
Temporarily enable logging of found namespaces
jonsimantov Jun 23, 2022
3a00f84
Fix command line
jonsimantov Jun 23, 2022
d05661a
Revert "Temporarily enable logging of found namespaces"
jonsimantov Jun 23, 2022
c574844
Add more implicit namespaces.
jonsimantov Jun 24, 2022
a9164ac
Add debug output
jonsimantov Jun 24, 2022
99cb123
Fix shell script
jonsimantov Jun 24, 2022
b7c4382
Fix regex to catch destructors properly.
jonsimantov Jul 1, 2022
2ed99dc
Also parse `RTTI Class Heirarchy Descriptor` on windows.
jonsimantov Jul 1, 2022
723e26e
Add '$' to top-level namespace search for Darwin.
jonsimantov Jul 11, 2022
2840d35
Fix renaming of Windows symbols with RTTI info, etc.
jonsimantov Jul 13, 2022
eca1452
Add tests for merge_libraries script.
jonsimantov Jul 13, 2022
f9a5846
Fix Windows symbol renaming for odd C++ symbols like RTTI descriptors
jonsimantov Jul 15, 2022
609e748
Ignore underscores as well to fix a Windows packging issue
jonsimantov Jul 15, 2022
5f747f7
Don't detect namespaces from external symbols.
jonsimantov Jul 18, 2022
6865231
Don't auto-add namespaces with underscores.
jonsimantov Jul 18, 2022
9e14cc0
Fix namespace detection to only rename top-level; add test.
jonsimantov Jul 18, 2022
74a4c07
Format code
jonsimantov Jul 18, 2022
cb1f66e
Clean up merge_libraries_test output
jonsimantov Jul 19, 2022
ca29af7
Fix python bug
jonsimantov Jul 19, 2022
f23c706
Add test for add_automatic_namespaces
jonsimantov Jul 19, 2022
1e10a48
Turn off debug output.
jonsimantov Jul 19, 2022
fd0c6d9
Add additional namespace to the list of ignored namespaces, for Windows.
jonsimantov Jul 19, 2022
eeb528b
Remove commented-out code
jonsimantov Jul 20, 2022
b2f7a06
Merge branch 'main' into bugfix/packaging_auto_hide_namespaces
jonsimantov Jul 20, 2022
af33afa
Move readme comment to upcoming release.
jonsimantov Jul 20, 2022
f543a28
Allow the list of allowed namespaces to be determined programatically.
jonsimantov Jul 20, 2022
3ee6f56
Fix path.
jonsimantov Jul 21, 2022
1783d7a
Fix typo
jonsimantov Jul 26, 2022
ba52fda
Fix regex
jonsimantov Jul 27, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 53 additions & 3 deletions .github/workflows/cpp-packaging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ on:

env:
# Packaging prerequisites
# Demumble 1.1.0 released Nov 13, 2018
demumbleVer: "1.1.0"
# Demumble version from March 22, 2022
demumbleVer: "df938e45c2b0e064fb5323d88b692d03b451d271"
# Use SHA256 for hashing files.
hashCommand: "sha256sum"
# Xcode version 13.3.1 is the version we build the SDK with.
Expand Down Expand Up @@ -94,6 +94,11 @@ jobs:
if: runner.os == 'macOS'
run: sudo xcode-select -s /Applications/Xcode_${{ env.xcodeVersion }}.app/Contents/Developer

- name: Setup python
uses: actions/setup-python@v2
with:
python-version: 3.7

- name: Fetch and build binutils
run: |
set +e
Expand Down Expand Up @@ -154,7 +159,7 @@ jobs:
with:
repository: nico/demumble
path: demumble-src
ref: v${{ env.demumbleVer }}
ref: ${{ env.demumbleVer }}

- name: build demumble
run: |
Expand Down Expand Up @@ -544,6 +549,9 @@ jobs:
cd sdk-src
python scripts/gha/install_prereqs_desktop.py --ssl boringssl
cd ..
if [[ $(uname) == "Darwin"* ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes an issue where Mac packaging was not done in parallel due to the "parallel" tool not being installed. Speeds up packaging on Mac.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

brew install parallel
fi

- name: postprocess and package built SDK
run: |
Expand Down Expand Up @@ -605,6 +613,48 @@ jobs:
name: firebase-cpp-sdk-${{ matrix.sdk_platform }}${{ matrix.suffix}}-package
path: firebase-cpp-sdk-${{ matrix.sdk_platform }}${{ matrix.suffix}}-package.tgz

- name: Check for unrenamed namespaces
shell: bash
run: |
set +e
if [[ "${{ matrix.sdk_platform }}" == "darwin" ]]; then
nm=bin/llvm-nm
elif [[ "${{ matrix.sdk_platform }}" == "windows" ]]; then
if [[ "${{ matrix.sdk_platform }}" == *"x64"* ]]; then
nm="bin/nm --target pe-x86-64"
else
nm="bin/nm --target pe-i386"
fi
else # sdk_platform = linux
nm=bin/nm
fi
${nm} `find firebase-cpp-sdk-*-package/libs/${{ matrix.sdk_platform }} -type f` | grep '^[0-9a-fA-F][0-9a-fA-F]* ' | cut -d' ' -f3- > raw_symbols.txt
if [[ "${{ matrix.sdk_platform }}" == "windows" ]]; then
cat raw_symbols.txt | sort | uniq | bin/demumble | grep '^[a-zA-Z_]*::' > cpp_symbols.txt
elif [[ "${{ matrix.sdk_platform }}" == "darwin" ]]; then
# remove leading _ on mach-o symbols
cat raw_symbols.txt | sort | uniq | sed 's/^_//' | bin/c++filt | grep '^[a-zA-Z_]*::' > cpp_symbols.txt
else # linux
cat raw_symbols.txt | sort | uniq | bin/c++filt | grep '^[a-zA-Z_]*::' > cpp_symbols.txt
fi
# cpp_symbols.txt contains a list of all C++ symbols, sorted and deduped
# get a list of all top-level namespaces (except system namespaces)
cat cpp_symbols.txt | cut -d: -f1 | sort | uniq > cpp_namespaces.txt
echo "List of all namespaces found:"
cat cpp_namespaces.txt
# Filter out renamed namespaces (f_b_*), standard namespaces, and the public namespace (firebase::).
# These are all specified in the respective scripts, package.sh and merge_libraries.py
echo $(sdk_src/build_scripts/desktop/package.sh -R)'*' > allow_namespaces.txt
sdk_src/build_scripts/desktop/package.sh -N >> allow_namespaces.txt
python sdk_src/scripts/print_allowed_namespaces.py >> allow_namespaces.txt
allow_namespaces=$(cat allow_namespaces=.txt | tr '\n' '|')
cat cpp_namespaces.txt | grep -vE "^(${allow_namespaces})$" > extra_namespaces.txt
# If there are any namespaces in this file, print an error.
if [[ -s extra_namespaces.txt ]]; then
echo '::error ::Unrenamed C++ namespaces in ${{ matrix.sdk_platform }}${{ matrix.suffix }} package:%0A'$(cat extra_namespaces.txt | tr '\n' ' ' | sed 's/ /%0A/g')
exit 1
fi

- name: cleanup build artifacts
if: |
(
Expand Down
50 changes: 18 additions & 32 deletions build_scripts/desktop/package.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ options:
-j, run merge_libraries jobs in parallel
-v, enable verbose mode
-L, use LLVM binutils
-R, print rename prefix and exit
-N, print allowed namespaces and exit
example:
build_scripts/desktop/package.sh -b firebase-cpp-sdk-linux -p linux -o package_out -v x86 -j"
}
Expand All @@ -42,6 +44,11 @@ use_llvm_binutils=0

readonly SUPPORTED_PLATFORMS=(linux windows darwin)

# String to prepend to all hidden symbols.
readonly rename_string=f_b_
# Comma-separated list of c++ namespaces to allow.
readonly allow_cpp_namespaces=firebase

abspath(){
if [[ -d $1 ]]; then
echo "$(cd "$1"; pwd -P)"
Expand All @@ -50,7 +57,7 @@ abspath(){
fi
}

while getopts "f:b:o:p:d:m:P:t:hjLv" opt; do
while getopts "f:b:o:p:d:m:P:t:NRhjLv" opt; do
case $opt in
f)
binutils_format=$OPTARG
Expand Down Expand Up @@ -94,6 +101,14 @@ while getopts "f:b:o:p:d:m:P:t:hjLv" opt; do
t)
tools_path=$OPTARG
;;
N)
echo "${allow_cpp_namespaces}" | tr ',' '\n'
exit 0
;;
R)
echo "${rename_string}"
exit 0
;;
h)
usage
exit 0
Expand Down Expand Up @@ -208,32 +223,6 @@ readonly deps_hidden_firebase_firestore="
*/firestore-build/*/grpc-build/third_party/re2/*${subdir}${prefix}re2.${ext}
"

# List of C++ namespaces to be renamed, so as to not conflict with the
# developer's own dependencies.
readonly -a rename_namespaces=(flatbuffers flexbuffers reflection ZLib bssl uWS absl google
base_raw_logging ConnectivityWatcher grpc
grpc_access_token_credentials grpc_alts_credentials
grpc_alts_server_credentials grpc_auth_context
grpc_channel_credentials grpc_channel_security_connector
grpc_chttp2_hpack_compressor grpc_chttp2_stream grpc_chttp2_transport
grpc_client_security_context grpc_composite_call_credentials
grpc_composite_channel_credentials grpc_core grpc_deadline_state
grpc_google_default_channel_credentials grpc_google_iam_credentials
grpc_google_refresh_token_credentials grpc_impl grpc_local_credentials
grpc_local_server_credentials grpc_md_only_test_credentials
grpc_message_compression_algorithm_for_level
grpc_oauth2_token_fetcher_credentials grpc_plugin_credentials
grpc_server_credentials grpc_server_security_connector
grpc_server_security_context
grpc_service_account_jwt_access_credentials grpc_ssl_credentials
grpc_ssl_server_credentials grpc_tls_credential_reload_config
grpc_tls_server_authorization_check_config GrpcUdpListener leveldb
leveldb_filterpolicy_create_bloom leveldb_writebatch_iterate strings
TlsCredentials TlsServerCredentials tsi snappy re2)

# String to prepend to all hidden symbols.
readonly rename_string=f_b_

readonly demangle_cmds=${tools_path}/c++filt,${tools_path}/demumble
if [[ ${use_llvm_binutils} -eq 1 ]]; then
readonly binutils_objcopy=${tools_path}/llvm-objcopy
Expand All @@ -260,14 +249,11 @@ merge_libraries_params=(
--binutils_objcopy_cmd=${binutils_objcopy}
--demangle_cmds=${demangle_cmds}
--platform=${platform}
--hide_cpp_namespaces=$(echo "${rename_namespaces[*]}" | sed 's| |,|g')
--auto_hide_cpp_namespaces
--ignore_cpp_namespaces="${allow_cpp_namespaces}"
)
cache_param=--cache=${cache_file}

if [[ ${platform} == "windows" ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer needed with the newer Demumble version.

# Windows has a hard time with strict C++ demangling.
merge_libraries_params+=(--nostrict_cpp)
fi
if [[ ${verbose} -eq 1 ]]; then
merge_libraries_params+=(--verbosity=3)
fi
Expand Down
6 changes: 6 additions & 0 deletions release_build_files/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,12 @@ workflow use only during the development of your app, not for publicly shipping
code.

## Release Notes
### Upcoming Release
- Changes
- General (Desktop): Fixed an issue with embedded dependencies that could
cause duplicate symbol linker errors in conjunction with other libraries
([#989](https://github.com/firebase/firebase-cpp-sdk/issues/989)).

### 9.3.0
- Changes
- General (Android,Linux): Fixed a concurrency bug where waiting for an
Expand Down
35 changes: 23 additions & 12 deletions scripts/merge_libraries.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,10 @@
"will autodetect target format. If you want to specify "
"different input and output formats, separate them with a comma.")

# Never rename 'std::' by default when --auto_hide_cpp_namespaces is enabled.
IMPLICIT_CPP_NAMESPACES_TO_IGNORE = {"std"}
# Never rename system namespaces by default when --auto_hide_cpp_namespaces is enabled.
IMPLICIT_CPP_NAMESPACES_TO_IGNORE = {"std", "type_info",
"__gnu_cxx", "stdext",
"cxxabiv1", "__cxxabiv1"}

DEFAULT_ENCODING = "ascii"

Expand Down Expand Up @@ -416,7 +418,7 @@ def get_top_level_namespaces(demangled_symbols):
# It will specifically not match second-level or deeper namespaces:
# matched_namespace::unmatched_namespace::Class::Method()
regex_top_level_namespaces = re.compile(
r"(?<![a-zA-Z0-9_])(?<!::)([a-zA-Z_][a-zA-Z0-9_]*)::(?=[a-zA-Z_])")
r"(?<![a-zA-Z0-9_])(?<!::)([a-zA-Z_~\$][a-zA-Z0-9_]*)::(?=[a-zA-Z_~` \$])")
found_namespaces = set()
for cppsymbol in demangled_symbols:
m = regex_top_level_namespaces.findall(cppsymbol)
Expand All @@ -441,6 +443,8 @@ def add_automatic_namespaces(symbols):
logging.debug("Scanning for top-level namespaces.")
demangled_symbols = [demangle_symbol(symbol) for symbol in symbols]
hide_namespaces = get_top_level_namespaces(demangled_symbols)
# strip out top-level namespaces beginning with an underscore
hide_namespaces = set([n for n in hide_namespaces if n[0] != "_"])
ignore_namespaces = set(FLAGS.ignore_cpp_namespaces)
ignore_namespaces.update(IMPLICIT_CPP_NAMESPACES_TO_IGNORE)
logging.debug("Ignoring namespaces: %s", " ".join(
Expand Down Expand Up @@ -533,25 +537,29 @@ def read_symbols_from_archive(archive_file):
return (defined_symbols, all_symbols)


def symbol_includes_cpp_namespace(cpp_symbol, namespace):
def symbol_includes_top_level_cpp_namespace(cpp_symbol, namespace):
"""Returns true if the C++ symbol contains the namespace in it.

This means the symbol is within the namespace, or the namespace as
an argument type, return type, template type, etc.

If FLAGS.strict_cpp == True, this will only return true if the namespace
is at the top level of the symbol.

Args:
cpp_symbol: C++ symbol to check.
namespace: Namespace to look for in the C++ symbol.
Returns:
True if the symbol includes the C++ namespace in some way, False otherwise.
True if the symbol includes the C++ namespace at the top level (or anywhere,
if FLAGS.strict_cpp == False), False otherwise.
"""
# Early out if the namespace isn't in the mangled symbol.
if namespace not in cpp_symbol:
return False
if not FLAGS.strict_cpp:
# If we aren't being fully strict about C++ symbol renaming,
# we can use this placeholder method.
if FLAGS.platform == "windows" and ("@%s@@" % namespace) in cpp_symbol:
if FLAGS.platform == "windows" and re.search(r"[^a-z_]%s@@" % namespace, cpp_symbol):
return True
elif (FLAGS.platform != "windows" and
("%d%s" % (len(namespace), namespace)) in cpp_symbol):
Expand All @@ -566,7 +574,8 @@ def symbol_includes_cpp_namespace(cpp_symbol, namespace):
return True
# Or, check if the demangled symbol has "namespace::" preceded by a non-
# alphanumeric character. This avoids a false positive on "notmynamespace::".
regex = re.compile("[^0-9a-zA-Z_]%s::" % namespace)
# Also don't allow a namespace :: right before the name.
regex = re.compile("[^0-9a-zA-Z_:]%s::" % namespace)
if re.search(regex, demangled):
return True

Expand Down Expand Up @@ -626,7 +635,7 @@ def rename_symbol(symbol):
new_symbol = symbol
if FLAGS.platform in ["linux", "android", "darwin", "ios"]:
for ns in FLAGS.hide_cpp_namespaces:
if symbol_includes_cpp_namespace(symbol, ns):
if symbol_includes_top_level_cpp_namespace(symbol, ns):
# Linux and Darwin: To rename "namespace" to "prefixnamespace",
# change all instances of "9namespace" to "15prefixnamespace".
# (the number is the length of the namespace name)
Expand All @@ -637,12 +646,12 @@ def rename_symbol(symbol):
new_renames[symbol] = new_symbol
elif FLAGS.platform == "windows":
for ns in FLAGS.hide_cpp_namespaces:
if symbol_includes_cpp_namespace(symbol, ns):
if symbol_includes_top_level_cpp_namespace(symbol, ns):
# Windows: To rename "namespace" to "prefixnamespace",
# change all instances of "@namespace@@" to "@prefixnamespace@@".
# change all instances of "[^a-z_]namespace@@" to "[^a-z]prefixnamespace@@",
# See https://msdn.microsoft.com/en-us/library/56h2zst2.aspx
new_ns = FLAGS.rename_string + ns
new_symbol = re.sub("@%s@@" % ns, "@%s@@" % new_ns, new_symbol)
new_symbol = re.sub(r"(?<=[^a-z_])%s@@" % ns, r"%s@@" % new_ns, new_symbol)
new_renames[symbol] = new_symbol
else:
if FLAGS.platform == "windows" and symbol.startswith("$LN"):
Expand Down Expand Up @@ -949,6 +958,7 @@ def main(argv):
# Scan through all input libraries for C++ symbols matching any of the
# hide_cpp_namespaces.
cpp_symbols = set()
all_defined_symbols = set()
for input_path in input_paths + additional_input_paths:
logging.debug("Scanning for C++ symbols in %s", input_path[1])
if os.path.abspath(input_path[1]) in _cache["symbols"]:
Expand All @@ -962,10 +972,11 @@ def main(argv):
cpp_symbols.update(all_symbols
if FLAGS.rename_external_cpp_symbols
else defined_symbols)
all_defined_symbols.update(defined_symbols)

# If we are set to scan for namespaces, do that now.
if FLAGS.auto_hide_cpp_namespaces:
add_automatic_namespaces(defined_symbols)
add_automatic_namespaces(all_defined_symbols)

for symbol in cpp_symbols:
if is_cpp_symbol(symbol):
Expand Down
Loading