Skip to content
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

Swift Package Manager support for CordovaLib #1154

Merged
merged 2 commits into from
Oct 6, 2021

Conversation

dpogue
Copy link
Member

@dpogue dpogue commented Sep 30, 2021

Platforms affected

iOS

Motivation and Context

This adds a Package.swift file to allow installing the CordovaLib library through Swift Package Manager.

This also cleans up our Cocoapods spec file to remove the need to explicitly list each header.

Description

Swift Package Manager requires all the public headers to be in an include folder, so all the headers from Classes/Public were moved to include/Cordova.

Also got rid of the precompiled header because SwiftPM doesn't know how to do that, and we were only precompiling Foundation and UIKit.

Xcode 13 also adds a new warning when framework headers are used with relative imports "myfile.h" rather than framework imports <myfile.h>. I've cleaned up the imports for all the public headers. (Note: some of the plugins use private headers, which must retain the relative import syntax)

Testing

  • Tested creating a new Swift project in Xcode, adding CordovaLib using SwiftPM and compiling successfully
  • Tested creating a new Objective C project in Xcode, adding CordovaLib using Cocoapods as a dynamic library and compiling successfully
  • Tested creating a new Objective C project in Xcode, adding CordovaLib using Cocoapods as a static library and compiling successfully
  • Existing test suite passes

Checklist

  • I've run the tests to see all new and existing tests pass

@dpogue dpogue added this to the 7.0.0 milestone Sep 30, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 1, 2021

Codecov Report

Merging #1154 (fc931bf) into master (4c81363) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1154   +/-   ##
=======================================
  Coverage   74.86%   74.86%           
=======================================
  Files          13       13           
  Lines        1723     1723           
=======================================
  Hits         1290     1290           
  Misses        433      433           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c81363...fc931bf. Read the comment docs.

These warnings show up in Xcode 13 with the recommended project
settings.
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

Code changes LGTM

I tested with default Cordova app.

  • Create Project
  • Platform Add
  • Prepare
  • Build
  • Run
    • Device is Ready passed
Cordova Packages:

    cli: 10.0.0
        common: 4.0.2
        create: 3.0.0
        lib: 10.0.0
            common: 4.0.2
            fetch: 3.0.1
            serve: 4.0.0

Project Installed Platforms:

    ios: 6.2.0

Environment:

    OS: macOS 11.6 (20G165) (darwin 20.6.0) x64
    Node: v14.18.0
    npm: 6.14.15

ios Environment:

    xcodebuild:
Xcode 13.0
Build version 13A233

@dpogue dpogue merged commit f12abcc into apache:master Oct 6, 2021
@dpogue dpogue deleted the swift-package-manager branch October 6, 2021 03:47
@erisu erisu mentioned this pull request Oct 6, 2021
5 tasks
gazben pushed a commit to apicore-engineering/cordova-ios that referenced this pull request Aug 26, 2022
* Fix warnings about quoted framework header paths

These warnings show up in Xcode 13 with the recommended project
settings.

* Restructure for Swift Package Manager support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants