-
Notifications
You must be signed in to change notification settings - Fork 49
IOS-5453: Add SPM support #333
Conversation
| s.pod_target_xcconfig = { | ||
| 'SWIFT_INCLUDE_PATHS' => '$(PODS_TARGET_SRCROOT)/TangemSdk/**', | ||
| 'OTHER_CFLAGS' => '-pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-shorten-64-to-32 -Wno-conditional-uninitialized -Wno-unused-function -Wno-long-long -Wno-overlength-strings -O3 -Wundef -Wreserved-identifier -fvisibility=hidden', | ||
| 'OTHER_CFLAGS' => '-Wpedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-shorten-64-to-32 -Wno-conditional-uninitialized -Wno-unused-function -Wno-long-long -Wno-overlength-strings -Wundef -Wreserved-identifier -O3 -fvisibility=hidden', |
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.
Вопрос, насколько нужно переносить все эти флаги в SPM?
Большинство из них как я понимаю для косметики (-Wsomething)
А нужную оптимизацию (-O2 или -O3) по идее SPM и так задаст в релизном билде
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.
без каких-то флагов были ошибки/ворнинги.
| # | ||
|
|
||
| Pod::Spec.new do |s| | ||
| s.name = 'TangemSdk' |
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.
В подспеке без изменений, просто немного поправил форматирование
| @@ -1,90 +0,0 @@ | |||
| /***************************************************************************************************** | |||
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.
С этим кодом очень забавная история - это по сути кодогенератор, юзаемый при сборке либы secp256k1 для того, чтобы нагенерить и записать в src/precomputed_ecmult.c видимо более оптимальные(?) для конкретной архитектуры векторы:
$ ./autogen.sh
$ mkdir ../build && cd ../build
$ ../secp256k1/configure
$ make precomp
bitcoin-core/secp256k1#1281 (comment)
If generate prebuilt sources manually, is it just like that:
gcc src/precompute_ecmult.c -o precompute_ecmult && ./precompute_ecmult
gcc src/precompute_ecmult_gen.c -o precompute_ecmult_gen && ./precompute_ecmult_gen
Именно поэтому в этом файле есть main (собственно в нем и происходит вся кодогенерация в другой сишный файл, src/precomputed_ecmult.c), чтобы его можно было запустить в процессе сборки либы.
Этот файл никак не используется в либе, а наличие в нем main вызывает ошибку дублирующих символов - поэтому я предлагаю его удалить.
Сейчас при подключении TangemSDK и через cocoapods и через SPM не производится это генерирование векторов при сборке.
Если нам в secp256k1 действительно есть необходимость в генерировании векторов при сборке (не знаю насколько это в целом будет работать при сборке под iOS на mac) - то надо или делать отдельный
build step и нем запускать этот файл или апгрейдить саму либу secp256k1, т.к. в bitcoin-core/secp256k1#988 ушли от необходимость делать precompute
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.
Думаю нужно будет написать нам внутренний гайд по обновлению secp256k1 либы
- переименование основного файла, чтобы ушли ошибки сборки через девелоперские поды
- правки отсюда a7cacbc
- удаление файла precompute_ecmult.c
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.
Добавил в notion
| // Copyright © 2020 Tangem AG. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation |
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.
Тут и в AccessCodeRepository.swift без явного импорта не собиралось в SPM, видимо более строгие правила насчет неявных импортов
| /// SPM preserves folder structure for resources, unlike Cocoapods. | ||
| /// Therefore, a full file path with all intermediate directories must be constructed. | ||
| private func filePath(forResource resource: String) -> String { |
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.
There are two ways to declare a resource:
A process rule: Applies platform specific rules to the resource such as compressing png files. If there are no specific rules for a resource the file is copied to the top-level directory of the bundle. You can specify a directory to have Xcode process the contents. The directory structure is not preserved.
A copy rule: Copies files untouched. If you pass a directory the contents are copied preserving the sub-directory structure.
copy rule к сожалению не настраиваемое, если не указывать каждый файлик по отдельности - то структура папок сохранится. Cocoapods же сваливает все в один бандл
Поэтому нужен этот хелпер
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.
Стринги все-таки пришлось перенести в отдельную папку на один уровень ниже, т.к. иначе приходится прописывать файл для каждой локали (en, ru, etc) в манифесте явно как ресурс - и соотв легко забыть это сделать, когда будет добавлен новый язык перевода
| IPHONEOS_DEPLOYMENT_TARGET = 12.0; | ||
| IPHONEOS_DEPLOYMENT_TARGET = 11.0; |
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.
Т.к. в спеке и манифесте все еще стоит min target 11.0 - то поправил это и в файле проекта (правда в Xcode 15 уже нет поддержки iOS 11)
| XCTAssertEqual(resultData.utf8String!.lowercased(), resultJsonData.utf8String!.lowercased()) | ||
| } | ||
|
|
||
| private func testMethodRequest(name: String) { |
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.
что-то не юзаемое
Было было идеально, заодно сможем что-то проверять таким образом |
tureck1y
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.
Обсудили, example добавим отдельно
IOS-5453
Старался сделать минимальные изменения структуры папок и файлов, в принципе получилось, единственное что пришлось перенести - это строки локализации.
В таргете юнит-тестов пришлось сделать правки из-за особенности работы SPM с ресурсами, но в итоге все тесты работают.
Example project по прежнему работает через поды, предлагаю обсудить - стоит ли перевести его на SPM или нет? Может вообще сделать два экзампла с общей логикой - один через pods, другой через SPM?