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

yubikey-manager: 4.0.9 -> 5.0.0 #198958

Merged
merged 2 commits into from
Nov 4, 2022
Merged

Conversation

afh
Copy link
Member

@afh afh commented Nov 1, 2022

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Lassulus
Copy link
Member

Lassulus commented Nov 1, 2022

Result of nixpkgs-review pr 198958 run on x86_64-linux 1

4 packages built:
  • gen-oath-safe
  • yubikey-manager
  • yubikey-manager-qt
  • yubioath-desktop

@Lassulus
Copy link
Member

Lassulus commented Nov 1, 2022

hmm, getting errors when running ykman-gui

qml: qrc:/qml/YubiKey.qml:208: TypeError: Cannot read property 'success' of undefined
"PyOtherSide error: Traceback (most recent call last):\n\n  File \"<string>\", line 1, in <module>\n\nNameError: name 'yubikey' is not defined\n"
qml: Function not found: 'yubikey.controller.refresh' (Traceback (most recent call last):

  File "<string>", line 1, in <module>

NameError: name 'yubikey' is not defined
)
qml: qrc:/qml/YubiKey.qml:208: TypeError: Cannot read property 'success' of undefined

@afh
Copy link
Member Author

afh commented Nov 3, 2022

Thanks for testing @Lassulus, much appreciated!

The major version change of yubikey-manager indicates breaking changes to prior versions, so it doesn't surprise me that yubikey-manager-qt, which has not seen a release or updates in about a year, doesn't work with yubikey-manager 5.x (see also yubikey-manager-qt#328).

First thing that comes to mind is to copy the current, i.e. prior to the changes in this PR, yubikey-manager/default.nix to yubikey-manager/40.nix, declare yubikey-manager40 in pkgs/top-level/all-packages based on 40.nix and make the yubikey-manager-qt derivation depend on that.

What do you think?
In case you agree that the changes outlined above are a viable path forward I'm happy to amend this PR accordingly.

@Lassulus
Copy link
Member

Lassulus commented Nov 3, 2022

sounds good, I guess yubikey-manager4 is good enough? since there was no 4.1.* ideally we would remove the old version anyways after the gui is updated.

@afh
Copy link
Member Author

afh commented Nov 3, 2022

Thanks for the quick response, @Lassulus.

I agree yubikey-manager4 is good enough and yes, once yubikey-manager-qt is updated to support yubikey-manager 5.x the yubikey-manager4 derivation should be removed.

The changes in this PR have been updated accordingly to earlier comments and I look forward to your review and further comments :)

@afh afh force-pushed the afh-yubikey-manager-update branch from 147c439 to 0bdd8c7 Compare November 4, 2022 06:23
@afh afh force-pushed the afh-yubikey-manager-update branch from 0bdd8c7 to 41af17c Compare November 4, 2022 10:15
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Nov 4, 2022
@Lassulus
Copy link
Member

Lassulus commented Nov 4, 2022

Result of nixpkgs-review pr 198958 run on x86_64-linux 1

4 packages built:
  • gen-oath-safe
  • yubikey-manager
  • yubikey-manager4
  • yubioath-desktop

@Lassulus Lassulus merged commit 987dc7a into NixOS:master Nov 4, 2022
@Lassulus
Copy link
Member

Lassulus commented Nov 4, 2022

thanks for taking care!

@afh afh deleted the afh-yubikey-manager-update branch November 4, 2022 16:12
@afh
Copy link
Member Author

afh commented Nov 4, 2022

Thanks for merging @Lassulus, much appreciated 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants