Skip to content

Upgrade grpc and abseil for SPM and cocoapods #9486

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 4 commits into from
Mar 25, 2022
Merged

Conversation

wu-hui
Copy link
Contributor

@wu-hui wu-hui commented Mar 21, 2022

No description provided.

@google-oss-bot
Copy link

1 Warning
⚠️ Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

@google-oss-bot
Copy link

google-oss-bot commented Mar 21, 2022

@google-oss-bot
Copy link

google-oss-bot commented Mar 21, 2022

Coverage Report 1

Affected Products

  • FirebaseFirestore-iOS-FirebaseFirestore.framework

    Overall coverage changed from ? (7613c44) to 86.61% (7b16f55) by ?.

    207 individual files with coverage change

    FilenameBase (7613c44)Merge (7b16f55)Diff
    any.nanopb.cc?0.00%?
    array_contains_any_filter.cc?100.00%?
    array_contains_filter.cc?100.00%?
    async_queue.cc?100.00%?
    auth_token.cc?100.00%?
    autoid.cc?100.00%?
    background_queue.cc?100.00%?
    bits.cc?100.00%?
    bound.cc?81.03%?
    bundle.nanopb.cc?0.00%?
    bundle_loader.cc?95.65%?
    bundle_reader.cc?90.29%?
    bundle_serializer.cc?91.84%?
    byte_stream_apple.mm?86.36%?
    byte_stream_cpp.cc?83.33%?
    byte_string.cc?80.30%?
    collection_reference.cc?100.00%?
    common.nanopb.cc?33.33%?
    comparison.cc?100.00%?
    connectivity_monitor.cc?100.00%?
    connectivity_monitor_apple.mm?51.92%?
    converters.mm?100.00%?
    database_id.cc?65.38%?
    database_info.cc?100.00%?
    datastore.cc?96.63%?
    delete_mutation.cc?86.36%?
    direction.cc?76.92%?
    document.cc?0.00%?
    document.nanopb.cc?94.78%?
    document_change.cc?62.50%?
    document_key.cc?93.06%?
    document_key_reference.cc?65.00%?
    document_reference.cc?100.00%?
    document_set.cc?87.30%?
    document_snapshot.cc?100.00%?
    empty.nanopb.cc?0.00%?
    error_apple.mm?92.31%?
    event_manager.cc?98.08%?
    exception.cc?23.68%?
    exception_apple.mm?89.66%?
    executor_libdispatch.mm?97.54%?
    executor_std.cc?97.71%?
    exponential_backoff.cc?100.00%?
    field_filter.cc?88.39%?
    field_index.cc?67.07%?
    field_mask.cc?100.00%?
    field_path.cc?98.16%?
    field_transform.cc?41.67%?
    filesystem_apple.mm?77.78%?
    filesystem_common.cc?81.67%?
    filesystem_posix.cc?81.82%?
    filter.cc?50.00%?
    FIRCollectionReference.mm?95.08%?
    FIRDocumentChange.mm?73.68%?
    FIRDocumentReference.mm?97.10%?
    FIRDocumentSnapshot.mm?96.84%?
    firebase_app_check_credentials_provider_apple.mm?92.93%?
    firebase_auth_credentials_provider_apple.mm?75.91%?
    firebase_metadata_provider_apple.mm?100.00%?
    firebase_metadata_provider_noop.cc?100.00%?
    firestore.cc?95.00%?
    firestore.nanopb.cc?37.12%?
    firestore_client.cc?98.57%?
    firestore_index_value_writer.cc?0.00%?
    FIRFieldPath.mm?90.24%?
    FIRFieldValue.mm?89.41%?
    FIRFilter.mm?100.00%?
    FIRFirestore.mm?90.69%?
    FIRFirestoreSettings.mm?84.21%?
    FIRFirestoreSource.mm?72.73%?
    FIRGeoPoint.mm?82.22%?
    FIRListenerRegistration.mm?100.00%?
    FIRLoadBundleTask.mm?80.70%?
    FIRQuery.mm?87.53%?
    FIRQuerySnapshot.mm?95.45%?
    FIRSnapshotMetadata.mm?100.00%?
    FIRTimestamp.m?79.35%?
    FIRTransaction.mm?96.85%?
    FIRWriteBatch.mm?100.00%?
    FSTFirestoreComponent.mm?97.83%?
    FSTUserDataReader.mm?94.36%?
    FSTUserDataWriter.mm?84.34%?
    geo_point.cc?65.00%?
    grpc_completion.cc?100.00%?
    grpc_connection.cc?77.08%?
    grpc_nanopb.cc?94.87%?
    grpc_root_certificate_finder_generated.cc?100.00%?
    grpc_stream.cc?99.01%?
    grpc_streaming_reader.cc?100.00%?
    grpc_unary_call.cc?100.00%?
    grpc_util.cc?100.00%?
    hard_assert.cc?100.00%?
    http.nanopb.cc?0.00%?
    index.nanopb.cc?0.00%?
    index_entry.cc?0.00%?
    in_filter.cc?100.00%?
    key_field_filter.cc?100.00%?
    key_field_in_filter.cc?100.00%?
    key_field_not_in_filter.cc?100.00%?
    latlng.nanopb.cc?76.92%?
    leveldb_bundle_cache.cc?76.00%?
    leveldb_document_overlay_cache.cc?97.17%?
    leveldb_index_manager.cc?86.11%?
    leveldb_key.cc?98.38%?
    leveldb_lru_reference_delegate.cc?94.31%?
    leveldb_migrations.cc?92.31%?
    leveldb_mutation_queue.cc?92.05%?
    leveldb_opener.cc?76.81%?
    leveldb_persistence.cc?88.89%?
    leveldb_remote_document_cache.cc?94.67%?
    leveldb_target_cache.cc?94.33%?
    leveldb_transaction.cc?98.11%?
    leveldb_util.cc?71.43%?
    load_bundle_task.cc?97.06%?
    local_documents_view.cc?100.00%?
    local_serializer.cc?84.96%?
    local_store.cc?100.00%?
    local_view_changes.cc?100.00%?
    log_apple.mm?93.33%?
    lru_garbage_collector.cc?91.34%?
    maybe_document.nanopb.cc?28.57%?
    memory_bundle_cache.cc?100.00%?
    memory_document_overlay_cache.cc?100.00%?
    memory_eager_reference_delegate.cc?100.00%?
    memory_index_manager.cc?41.38%?
    memory_lru_reference_delegate.cc?95.41%?
    memory_mutation_queue.cc?98.17%?
    memory_persistence.cc?95.65%?
    memory_remote_document_cache.cc?100.00%?
    memory_target_cache.cc?100.00%?
    message.cc?100.00%?
    mutable_document.cc?64.49%?
    mutation.cc?88.17%?
    mutation.nanopb.cc?74.19%?
    mutation_batch.cc?86.89%?
    mutation_batch_result.cc?52.17%?
    nanopb_util.cc?100.00%?
    not_in_filter.cc?100.00%?
    object_value.cc?100.00%?
    online_state_tracker.cc?100.00%?
    ordered_code.cc?93.41%?
    order_by.cc?50.00%?
    overlay.cc?100.00%?
    patch_mutation.cc?100.00%?
    path.cc?100.00%?
    precondition.cc?75.68%?
    pretty_printing.cc?83.33%?
    proto_sizer.cc?72.73%?
    query.cc?96.77%?
    query.nanopb.cc?82.24%?
    query_core.cc?95.65%?
    query_engine.cc?96.49%?
    query_listener.cc?100.00%?
    query_listener_registration.cc?100.00%?
    query_snapshot.cc?83.16%?
    reader.cc?100.00%?
    reference_set.cc?88.89%?
    remote_event.cc?96.38%?
    remote_objc_bridge.cc?90.23%?
    remote_store.cc?90.35%?
    resource.nanopb.cc?0.00%?
    resource_path.cc?100.00%?
    schedule.cc?100.00%?
    secure_random_arc4random.cc?100.00%?
    serializer.cc?87.16%?
    server_timestamp_util.cc?97.06%?
    settings.cc?0.00%?
    set_mutation.cc?89.36%?
    snapshots_in_sync_listener_registration.cc?100.00%?
    snapshot_metadata.cc?100.00%?
    snapshot_version.cc?83.33%?
    status.cc?73.05%?
    status.nanopb.cc?68.75%?
    statusor.cc?80.00%?
    status_apple.mm?96.61%?
    status_errno.cc?30.25%?
    stream.cc?98.44%?
    strerror.cc?100.00%?
    string_apple.cc?100.00%?
    string_format.cc?93.94%?
    string_util.cc?100.00%?
    struct.nanopb.cc?5.06%?
    sync_engine.cc?95.20%?
    target.cc?93.62%?
    target.nanopb.cc?45.95%?
    target_data.cc?40.63%?
    target_id_generator.cc?100.00%?
    task.cc?94.78%?
    timestamp.cc?94.00%?
    timestamp.nanopb.cc?100.00%?
    timestamp_internal.cc?53.33%?
    transaction.cc?89.17%?
    transaction_runner.cc?100.00%?
    transform_operation.cc?76.13%?
    user.cc?100.00%?
    user_data.cc?96.30%?
    value_util.cc?93.58%?
    verify_mutation.cc?13.33%?
    view.cc?97.56%?
    view_snapshot.cc?76.92%?
    watch_change.cc?83.33%?
    watch_stream.cc?90.70%?
    wrappers.nanopb.cc?8.33%?
    write.nanopb.cc?60.25%?
    writer.cc?96.97%?
    write_batch.cc?91.89%?
    write_stream.cc?90.14%?

  • FirebaseFirestore-iOS-FirebaseFirestoreSwift.framework

    Overall coverage changed from ? (7613c44) to 45.83% (7b16f55) by ?.

    21 individual files with coverage change

    FilenameBase (7613c44)Merge (7b16f55)Diff
    CodablePassThroughTypes.swift?100.00%?
    CollectionReference+AsyncAwait.swift?93.75%?
    CollectionReference+WriteEncodable.swift?100.00%?
    DocumentID.swift?80.00%?
    DocumentReference+Codable.swift?50.00%?
    DocumentReference+ReadDecodable.swift?0.00%?
    DocumentReference+WriteEncodable.swift?100.00%?
    DocumentSnapshot+ReadDecodable.swift?80.00%?
    ExplicitNull.swift?90.48%?
    FieldValue+Encodable.swift?100.00%?
    Firestore+AsyncAwait.swift?96.67%?
    FirestoreDecoder.swift?45.89%?
    FirestoreEncoder.swift?60.41%?
    FirestoreQuery.swift?0.00%?
    FirestoreQueryObservable.swift?0.00%?
    GeoPoint+Codable.swift?100.00%?
    QueryPredicate.swift?0.00%?
    ServerTimestamp.swift?96.97%?
    Timestamp+Codable.swift?100.00%?
    Transaction+WriteEncodable.swift?100.00%?
    WriteBatch+WriteEncodable.swift?100.00%?

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/CqG636W5yC.html

@dennycd
Copy link

dennycd commented Mar 22, 2022

abseil build break to be fixed via firebase/abseil-cpp-SwiftPM#9

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

  • Why aren't the abseil versions consistent between CocoaPods and SPM?
  • Why is the CocoaPods abseil version locked to a patch release but SPM enables patch updates?
  • Why do we allow patch updates for abseil but not gRPC
  • Is the CI build warning in abseil fixable:
  • - WARN  | xcodebuild:  /Users/runner/Library/Developer/Xcode/DerivedData/App-ffzsphyzsoubipeuqfnlcfzcgour/Build/Products/Release-iphonesimulator/abseil/absl.framework/Headers/strings/str_split.h:517:7: warning: Forwarding reference passed to std::move(), which may unexpectedly cause lvalues to be moved; use std::forward() instead [bugprone-move-forwarding-reference]
    
    • WARN | xcodebuild: /Users/runner/Library/Developer/Xcode/DerivedData/App-ffzsphyzsoubipeuqfnlcfzcgour/Build/Products/Release-iphonesimulator/abseil/absl.framework/Headers/strings/str_split.h:542:7: warning: Forwarding reference passed to std::move(), which may unexpectedly cause lvalues to be moved; use std::forward() instead [bugprone-move-forwarding-reference]

@wu-hui
Copy link
Contributor Author

wu-hui commented Mar 23, 2022

  • Why aren't the abseil versions consistent between CocoaPods and SPM?

Abseil on cocoapods is behind, I am using the latest I can use. @dennycd can you help?

  • Why is the CocoaPods abseil version locked to a patch release but SPM enables patch updates?
  • Why do we allow patch updates for abseil but not gRPC

I am not sure why abseil is fixed on version. The history is lost..I am guess we have been bitten before by absl. I am enabling it for now...let's see what happens.

gRPC being fixed is a mistake.

  • Is the CI build warning in abseil fixable:

Will check.

@wu-hui wu-hui closed this Mar 23, 2022
@wu-hui wu-hui reopened this Mar 23, 2022
@dennycd
Copy link

dennycd commented Mar 24, 2022

Abseil on cocoapods is behind, I am using the latest I can use. @dennycd can you help?

looks like abseil-cpp-SwiftPM's tag (0.20220203.0 and up) are indeed using the same abseil version as latest cocoapod (20211102.0), just that the tag names from abseil-cpp-SwiftPM doesn't exactly match abseil's release tags.

We can either keep the current tag scheme for now, or re-tag abseil-cpp-SwiftPM to match that from cocoapod (e.g. 0.20211102.0). Hopefully once we resolve abseil SPM & Cocoapod dependency w/ abseil team, we can start using a consistent release tag from the same repo

@morganchen12
Copy link
Contributor

This PR also includes grpc/grpc#27300, which resolves firebase/quickstart-ios#1099.

@paulb777 paulb777 closed this Mar 25, 2022
@paulb777 paulb777 reopened this Mar 25, 2022
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

LGTM on green

@wu-hui wu-hui merged commit 9d51e8a into master Mar 25, 2022
@wu-hui wu-hui deleted the wuandy/GrpcSpmPod144 branch March 25, 2022 14:48
wu-hui added a commit that referenced this pull request Mar 25, 2022
wu-hui added a commit that referenced this pull request Mar 25, 2022
@firebase firebase locked and limited conversation to collaborators Apr 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants