Distribute multiple packages divided by features with Swift Package Manager

onagat commented 4 years ago

In this repo., we have codes for various cards.

It would be great if users can use a specific framework separately (say, for Transit, Drivers Licence)😍

How can we do this?

onagat commented 4 years ago

これは、使えそうでしょうか？ Does this work?

https://forums.swift.org/t/how-to-produce-multiple-libraries-from-one-package/18478

treastrain commented 4 years ago

@onagat

ありがとうございます。Swift Package Manager であればその方法がよいかもしれません。 ~~Carthage ではまた別な方法を考える必要がありそうです。~~ こちらの Issue では SPM のみを対象となっていました。

Swift Package Manager では Resources が現在使えない状態ですので、ローカライズで問題が発生します。まずはこちらの回避策を講じてから、Swift Package Manager だけでも使用するコードを分割して App に組み込めるように考えたいと思います。

onagat commented 4 years ago

@treastrain あぁ、確かに！交通系だけでも、SPM 使えると良いかもしれません☺️ 私は、SPM 派なので、ちょっと、明日あたり時間あるとき、実装試してみようかと思います。 Carthage でもできると良いですが！

issue は適当に、変更したり、統合したり、諸々してください🙇‍♂️

treastrain commented 4 years ago

いま考えているのは、これまでローカライズのために用いていた func localizedString(key: String) のアクセス修飾子を internal から open に変更し、JapanNFCReader を init する際に override してもらう方法です。ただ、もう少しよい実装方法があるのではないかとも考えていますので、ローカライズを行っている他のライブラリの実装等も探しながら模索したいと思っています。

https://github.com/treastrain/TRETJapanNFCReader/blob/b281d527349f42bf2cc8819294162aad861098f4/TRETJapanNFCReader/JapanNFCReader.swift#L101-L103

onagat commented 4 years ago

こういうの、お互いのアイデアをこういう issue の場で議論できると、良い考えが生まれそうですね！せっかくの、issue なので、良ければそういうのに活用しましょう！

ちょっと理解した後、返事します！前おっしゃってたbundle 問題は、あの後勉強して理解しました！

treastrain commented 4 years ago

I created a branch: divided-by-features-spm from develop

treastrain commented 4 years ago

分割自体は終了し、もともと存在する Swift Package Manager で Assets が使えない問題により運転免許証の JIS X 0208 からデコードできない事象と、ローカライズされた文言が表示されない事象が残っていますが、.ipa ファイルにしたときに軽量化されることを確認しました。

以上のように TRETJapanNFCReader の全てを追加した場合と、交通系ICのための TRETJapanNFCReader/FeliCa/TransitIC のみを追加した場合とで 342KB → 90KB となりました。

今後は、このままでは CocoaPods と Carthage での使用に問題があるため、そちらに対応することを優先とし、また import 文に変化があるため破壊的変更とならないように修正を加えます。

以下、.ipa 書き出し時の情報とサンプルコードです。

開発機

macOS Catalina 10.15.2 (19C57)
Xcode Version 11.3 (11C29)
Apple Swift version 5.1.3 (swiftlang-1100.0.282.1 clang-1100.0.33.15)

ターゲット

Target: iOS 13.2 (iPhone, iPad)

アーカイブ

Method of distribution: Ad Hoc
App Thining: None
Rebuild from Bitcode: True
Include manifest for over-the-air installation: False
Re-sign: Automatically manage signing

コード(all.ipa と only_transitIC.ipa のどちらも同一)

import UIKit
import TRETJapanNFCReader_FeliCa
import TRETJapanNFCReader_FeliCa_TransitIC

class ViewController: UIViewController, FeliCaReaderSessionDelegate {

    var reader: TransitICReader!

    override func viewDidLoad() {
        super.viewDidLoad()

        self.reader = TransitICReader(viewController: self)
        self.reader.get(itemTypes: [.balance])
    }

    func feliCaReaderSession(didRead feliCaCard: FeliCaCard) {
        let transitICCard = feliCaCard as! TransitICCard
        print(transitICCard.data.balance ?? "nil")
    }

    func japanNFCReaderSession(didInvalidateWithError error: Error) {

    }
}

onagat commented 4 years ago

なかなか大きい作業、本当にありがとうございます😊 かなり軽くなっておりますね！貢献できておらず申し訳ありません🙇‍♂️

アセットについては、利用可能にするには、辞書型か何かで、コードに埋め込むしかアイデアがないですね。気持ち悪い気もします。

treastrain commented 4 years ago

今のところ、JIS X 0208 についてはすべてを enum にしてしまうのはどうか…と考えています。ただ、これまでどおり TXT から変換する方法と、enum にあらかじめ定義したものから変換する方法とでパフォーマンスに問題がないかどうかについて調査する必要があると考えています。

onagat commented 4 years ago

コードを確認しましたが、膨大な分割作業ありがとうございました🙇‍♂️

辞書をコードに埋め込むと下記の感じでしょうか？

let tableFromJISX0208ToUnicode: [Data : Data] = [
 0x2121: 0x3000,  # IDEOGRAPHIC SPACE
 0x2122: 0x3001,  # IDEOGRAPHIC COMMA
 0x2123: 0x3002   # IDEOGRAPHIC FULL STOP
]

treastrain commented 4 years ago

おそらくですが Dictionary にしてしまうとかなりのメモリを確保することになるのではないか…と思います。また、

let space: Data = tableFromJISX0208ToUnicode[0x2121]! // 0x3000

のように毎度 Optional を外すことになりそうなので、enum で列挙できないかなと考えています。

onagat commented 4 years ago

なるほど。Optional を外すあたりを考えると、enum の方がコードが綺麗な可能性がありますね！☺️ 英語で調べてもこの辺の情報は全然ないんですね。

辞書だと、データが 6000文字 x 4bytes = 24KB なので、辞書が追加で消費するメモリも加えて 40kB 程度でしょうか。私の知識では、ハッシュなどの性質で、辞書は 10000 を大幅に超えると、ありえないメモリ消費量やパフォーマンスになることがあると思っております。6000だと許容範囲とする考え方もあり得ると思います。

treastrain commented 4 years ago

自分の今の実装が TXT ファイル読み込み → 辞書に変換 → key に JIS X 0208 の Data を当てて value を取り出すであったことを完全に忘れていました…。これであれば、そもそも Dictionary をあらかじめ定義しておくことに比べれば、それよりパフォーマンスが悪くなることはありませんね…。

Swift の標準で、通常 Data から String に変換するときにに用いる init?(data: Data, encoding: String.Encoding) (Apple Developer Documentation) に近い仕様にできるとなお良いので、これがどのように実装されているかについても少し確認してみることにします…。

onagat commented 4 years ago

本当に数多くのカードを扱われているので、免許証部分がもう記憶から薄れていたんですね笑☺️

それは、良い情報ですね😝！私も確認してみます！

ローカライズの問題点を確認したいのですが、SPM にリソースをつけられないのが問題なのでしょうか？お教えくださいませ🙇‍♂️

treastrain commented 4 years ago

@onagat

ローカライズを行うときに、NSLocalizedString(_:tableName:bundle:value:comment:) (Apple Developer Documentation) を用いて翻訳を行いますが、ここで指定する bundle で Localizable.strings がある Bundle を指定しなければなりません。Swift Package Manager では、ライブラリ自身がいる Bundle を取得できないため、Localizable.strings にたどり着けないというように認識しています。

このあたりを参照していただければと思います。 Swift PM, Bundles and Resources - Development / Package Manager - Swift Forums

Qs-F commented 4 years ago

Swiftにはあまり明るくないので、的はずれな意見でしたらすみません。

私はこのプロジェクトについては、Firebaseのようなやり方に学ぶのが最も適していると考えています。
つまり、 .swift の import xxxを増やして機能を拡張するJavaのような方法ではなく、 CocoaPods(のpod)やCarthage(のbinary distribution)で切り分けを行うのがSwiftらしいのかなと感じています。いかがでしょうか。

追記: このコメントのコンテキストはこのツイートですhttps://twitter.com/treastrain/status/1210897635906768896

IMO, referring to and following what Firebase does seems reasonable.
In detail, separating and using packages via CocoaPods (pod) or Carthage (binary distribution) is better, rather than by adding redundant import xxx lines into .swift file.
Although the latter way is commonly used in Java, I don't feel it's swiftish way.

Here is the context of this comment: https://twitter.com/treastrain/status/1210897635906768896

Qs-F commented 4 years ago

あ、すみませんこれSPMの話限定だったんですね…すみませんよく読んでなかったです

treastrain commented 4 years ago

@Qs-F

ありがとうございます。私も import 文は import TRETJapanNFCReader の1つのみにしたいと考えています。Objective-C 等で書かれている Firebase 群はこれが実現できています。Carthage や CocoaPods で機能別に分割されたライブラリを提供する際にはこれを目指します。

Thanks. I also want to have only one the statement, import TRETJapanNFCReader. Firebase libraries written in Objective-C etc. can achieve this. I aim for this when publishing the libraries split by feature at Carthage and CocoaPods.

treastrain commented 4 years ago

Swift Package Manager のための機能別分割はひとまずできたため、残された JIS X 0208 からのデコードに関する問題と、ローカライズに関する問題を別な Issue に分けることとします。

treastrain / TRETJapanNFCReader

Distribute multiple packages divided by features with Swift Package Manager #24