-
Notifications
You must be signed in to change notification settings - Fork 90
Feat: Qt keycard support with nfc and pcsc #19545
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
base: master
Are you sure you want to change the base?
Conversation
This commit adds the status-keycard-qt submodule and updates the build configuration to support it. In this initial version the `status-keycard-qt` option can be enabled by using the `USE_STATUS_KEYCARD_QT` flag. - desktop: `USE_STATUS_KEYCARD_QT` will enable/disable the qt implementation. By default the go version is used - ios: `USE_STATUS_KEYCARD_QT` will enable/disable the qt implementation - android: the qt implementation is enabled by default
✔️ status-desktop/prs/linux/x86_64/tests-ui/PR-19545#1 🔹 ~13 min 🔹 8bb7613 🔹 📦 tests/ui package |
✔️ status-desktop/e2e/prspr19545 🔹 ~15 min 🔹 8bb7613 🔹 📦 tests/e2e package |
✔️ status-desktop/prs/linux/x86_64/tests-ui/PR-19545#2 🔹 ~14 min 🔹 450a51f 🔹 📦 tests/ui package |
✔️ status-desktop/e2e/prspr19545 🔹 ~16 min 🔹 450a51f 🔹 📦 tests/e2e package |
✔️ status-desktop/prs/android/arm64/package/PR-19545#3 🔹 ~11 min 🔹 77b27f38 🔹 📦 android/arm64 package |
✔️ status-desktop/prs/linux/x86_64/tests-ui/PR-19545#3 🔹 ~13 min 🔹 adadfcd 🔹 📦 tests/ui package |
✔️ status-desktop/e2e/prspr19545 🔹 ~32 min 🔹 adadfcd 🔹 📦 tests/e2e package |
✔️ status-desktop/e2e/prs-windowsPR19545 🔹 ~23 min 🔹 adadfcd 🔹 📦 tests/e2e-windows package |
✔️ status-desktop/prs/android/arm64/package/PR-19545#4 🔹 ~11 min 🔹 e6338a7a 🔹 📦 android/arm64 package |
✔️ status-app/prs/linux/x86_64/tests-ui/PR-19545#4 🔹 ~1 hr 6 min 🔹 7e8edd3 🔹 📦 tests/ui package |
✔️ status-app/prs/linux/x86_64/tests-nim/PR-19545#5 🔹 ~10 min 🔹 f23b896 🔹 📦 tests/nim package |
✔️ status-app/prs/linux/x86_64/tests-ui/PR-19545#5 🔹 ~14 min 🔹 f23b896 🔹 📦 tests/ui package |
Jenkins BuildsClick to see older builds (45)
|
| NIM_FLAGS+=(-d:debug -d:nimTypeNames) | ||
| else | ||
| NIM_FLAGS+=(-d:release -d:lto -d:production) | ||
| fi |
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.
Just realized it would be nice to build in a different dir for debug/release; wdyt?
| QT_MAJOR=$(QT_MAJOR) \ | ||
| DEVELOPMENT_TEAM="$(DEVELOPMENT_TEAM)" \ | ||
| FLAG_KEYCARD_ENABLED=$(FLAG_KEYCARD_ENABLED) \ | ||
| $(APP_SCRIPT) $(HANDLE_OUTPUT) |
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.
Those 2 if/else branches are the same :)
| KEYCARD_LIB := $(STATUSKEYCARD_QT_LIB) | ||
| KEYCARD_LIBDIR := $(STATUSKEYCARD_QT_LIBDIR) | ||
| KEYCARD_LINKNAME := status-keycard-qt | ||
| KEYCARD_DYLIB_NAME := libstatus-keycard-qt.dylib |
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.
That looks like a macOS specific libname
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.
Linux will look like "libstatus-keycard-qt.so"
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.
Should use a variant of STATUSKEYCARD_QT_LIB?
| KEYCARD_LIB := $(STATUSKEYCARDGO) | ||
| KEYCARD_LIBDIR := $(STATUSKEYCARDGO_LIBDIR) | ||
| KEYCARD_LINKNAME := keycard | ||
| KEYCARD_DYLIB_NAME := libkeycard.dylib |
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.
Same here
|
|
||
| # Allow using local status-keycard-qt for development | ||
| STATUS_KEYCARD_QT_DIR ?= vendor/status-keycard-qt | ||
| KEYCARD_QT ?= "" |
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.
What is this variable supposed to do/contain?
| ## status-keycard-qt (Qt/C++ based keycard library) | ||
| ## | ||
|
|
||
| # Allow using local status-keycard-qt for development |
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.
Would be nice to document this; ie how to build the desktop client with this new lib (BUILDING.md)
What does the PR do
Add https://github.com/status-im/status-keycard-qt lib as a keycard integration option.
This commit adds the status-keycard-qt submodule and updates the build configuration to support it.
In this initial version the
status-keycard-qtoption can be enabled by using theUSE_STATUS_KEYCARD_QTflag.USE_STATUS_KEYCARD_QTwill enable/disable the qt implementation. By default the go version is usedUSE_STATUS_KEYCARD_QTwill enable/disable the qt implementation. On IOS the developer experience is quite bad and I've tried not to make it worse here. When compiling the app with keycard support a paid apple developer account is needed to sign the app. For this reason theUSE_STATUS_KEYCARD_QTis disabled by default in the mobile makefile. People with apple accounts can use theDEVELOPMENT_TEAMflag to configure the signing before running the app.USE_STATUS_KEYCARD_QTAcceptance criteria
NOTE: Tests can be done in the final PR #19549