-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
intial unit test and impl for ssh-keygen simulator #1432
base: master
Are you sure you want to change the base?
Conversation
Probably not worth merging yet since it doesn't actually to any check. So I've put it in draft mode. But good preview of what's to come soon so you can point out any improvements and changes that make sense. |
@jelmer Pick your poison: Soon I'm going to be adding a line of
from https://gitlab.com/perm.pub/hidos/-/blob/923691a51921de59250e80f7fd82fe40fa895637/hidos/sshsiglib/ssh_keygen.py#L1 And I think there are few other benefits. BUT if you think it's better to not |
Why do we need that? Dulwich is python >= 3.9, so annotations shouldn't require a "from future import" |
It is still needed for recursive/self-referential type hints. For instance, in the unquoted |
I'm usually in a container with Python 3.12 and the choice still is needed in 3.12. |
Interesting ... I just discovered another benefit to using ruff requests using the |
@jelmer OK, this is an actual implementation of |
I figured I would just throw all the code in to one file, to keep it simple within the bigger picture of dulwich. However, there are a number of natural splits and layers to this implementation. This single file could naturally be split into multiple and placed within a subdirectory/submodule for all this sshsig/ssh_keygen stuff. |
Another option here is to stay consistent with the rest of the code and ignore this suggestion from ruff by adding |
* core functionality of ssh-keygen -Y check-novalidate * unit test for ssh-keygen check-novalidate functionality * compat test to double check against real ssh-keygen exec
76083a2
to
bc1b571
Compare
@jelmer OK, I believe all outstanding issues are resolved now. I've also rebased and squashed. There are some minor trade-offs between using Since the key two functions at the end of sshsig.py aren't getting called by anything in Dulwich yet, I'm not sure whether it makes sense to merge this into the master branch. But to me it looks fine to merge now. From what I know now, there isn't much reason for this code to change. Additional SSH signing functionality is all built on top of this PR and shouldn't require much modification of this PR code. |
trivial always fail impl of check-novalidate subcmd