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.

Sign up for GitHub

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

Jump to bottom

1st nkstorecli integration as module #1

Closed
wants to merge 1 commit into from
Closed

1st nkstorecli integration as module #1

wants to merge 1 commit into from

Conversation

daringer
Copy link
Collaborator

  • module is currently made available as binary
  • hack-ish integration in modules/nkstorecli
  • currently only configured for board: qemu-coreboot

@szszszsz
Copy link
Member

Connected:

szszszsz
szszszsz approved these changes Jul 13, 2020
View reviewed changes
Copy link
Member

@szszszsz szszszsz left a comment

Choose a reason for hiding this comment

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

LGTM

@szszszsz
Copy link
Member

Hi @alex-nitrokey !
Could you take a quick look?

alex-nitrokey
alex-nitrokey reviewed Jul 30, 2020
View reviewed changes
Copy link

@alex-nitrokey alex-nitrokey left a comment

Choose a reason for hiding this comment

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

I did not test actually building this PR. Please let me know if I should :)

Did you already check the reproducibility or is it something not of interest right now? We should before creating a PR in osresearch/heads ...

build/.gitignore Outdated Show resolved Hide resolved
install/.gitignore Outdated Show resolved Hide resolved
modules/libhidapi-libusb Show resolved Hide resolved
modules/libhidapi-libusb Outdated Show resolved Hide resolved
modules/libnitrokey
cd build && \
$(CROSS_TOOLS) cmake .. -DNO_LOG=ON -DBUILD_SHARED_LIBS=ON -DCOMPILE_TESTS=OFF -DCMAKE_INSTALL_PREFIX=/ -DCMAKE_INSTALL_LIBDIR=lib -DCMAKE_BUILD_TYPE=MinSizeRel $(cmake_cross)

# install "by-hand" as INSTALL_PREFIX is not working as expected
Copy link

Choose a reason for hiding this comment

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

Can we fix this in libnitrokey @szszszsz ?

Copy link
Collaborator Author

@daringer daringer Aug 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

yes, not due to my hacks ... but would also suggest to fix that or at least disable the cmake configuration for it ...

Copy link
Member

Choose a reason for hiding this comment

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

Surely we can make a custom installation command instead, so we would hide it here.
What's the actual problem though? @daringer could you make a ticket on the libnk project?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

jup will do

modules/musl-cross Show resolved Hide resolved
modules/musl-cross Show resolved Hide resolved
modules/nkstorecli Show resolved Hide resolved
tlaurion added a commit to tlaurion/heads that referenced this pull request Aug 14, 2020
@tlaurion
Minimizing changes from Nitrokey#1 to at least reuse the musl-cross c…
86a2418 
…ache on CI. Removed x230-hotp-verification changes to hack it more up to review Nitrokey PR against heads' current master
tlaurion added a commit to tlaurion/heads that referenced this pull request Aug 14, 2020
@tlaurion
Minimizing changes from Nitrokey#1 to at least reuse the musl-cross c…
ce2b464 
…ache on CI. Removed x230-hotp-verification changes to hack it more up to review Nitrokey PR against heads' current master
tlaurion added a commit to tlaurion/heads that referenced this pull request Aug 14, 2020
@tlaurion
Minimizing changes from Nitrokey#1 to at least reuse the musl-cross c…
a562951 
…ache on CI. Removed x230-hotp-verification changes to hack it more up to review Nitrokey PR against heads' current master
tlaurion added a commit to tlaurion/heads that referenced this pull request Aug 22, 2020
@tlaurion
Minimizing changes from Nitrokey#1 to at least reuse the musl-cross c…
76d3d20 
…ache on CI. Removed x230-hotp-verification changes to hack it more up to review Nitrokey PR against heads' current master
@tlaurion tlaurion mentioned this pull request Aug 22, 2020
Closed
@daringer
Copy link
Collaborator Author

closing in favor of: linuxboot#816

@daringer daringer closed this Aug 30, 2020
tlaurion added a commit to tlaurion/heads that referenced this pull request Oct 25, 2020
@tlaurion
Minimizing changes from Nitrokey#1 to at least reuse the musl-cross c…
7c3dadc 
…ache on CI. Removed x230-hotp-verification changes to hack it more up to review Nitrokey PR against heads' current master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szszszsz szszszsz szszszsz approved these changes

@alex-nitrokey alex-nitrokey alex-nitrokey left review comments

Assignees

@alex-nitrokey alex-nitrokey

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@daringer @szszszsz @alex-nitrokey