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

tkey-ssh-agent: check agent-path existance also on win; show notification #80

Merged
merged 3 commits into from
Jan 2, 2024
Merged

tkey-ssh-agent: check agent-path existance also on win; show notification #80

merged 3 commits into from
Jan 2, 2024

Conversation

quite
Copy link
Contributor

@quite quite commented Apr 24, 2023 •
edited
Loading

No description provided.

@quite quite mentioned this pull request Apr 24, 2023
Closed
@quite quite changed the title tkey-ssh-agent: check agent-path existance also on win; show notifica… tkey-ssh-agent: check agent-path existance also on win; show notification Apr 24, 2023
@dehanj dehanj self-requested a review September 11, 2023 12:50
@mchack-work mchack-work self-requested a review October 3, 2023 14:41
@dehanj
Copy link
Member

dehanj commented Jan 2, 2024 •
edited
Loading

Rearranged and fix an issue that, if an agent already is running with the same agentPath, this instance would remove the agentPath for the already running agent when exiting.

@dehanj dehanj force-pushed the notify-already-running branch 2 times, most recently from 79e9df1 to dc44035 Compare January 2, 2024 14:25
@dehanj
Copy link
Member

dehanj commented Jan 2, 2024 •
edited
Loading

Rebased on main, and restored gotools/go-winres which we accidentally removed when we moved golangci-lint to GH actions.

Also tested the results on windows and mac.

dehanj
dehanj previously approved these changes Jan 2, 2024
View reviewed changes
@dehanj dehanj self-requested a review January 2, 2024 14:42
@dehanj dehanj dismissed their stale review January 2, 2024 14:43

Seems like the socket is not erased on close now. Will investigate.

@dehanj
Copy link
Member

dehanj commented Jan 2, 2024

The problem seems to be that the exit-function including the agentPath-removal has been moved below
signer := NewSigner(devPath, speed, enterUSS, fileUSS, pinentry, exit), which takes the exit-function as an input. So when one does Ctrl-C it is not removing the agentPath. The error is present in main, but not in v0.0.6 release. Will fix tomorrow.

@dehanj dehanj merged commit 8599e41 into main Jan 2, 2024
2 checks passed
@dehanj dehanj deleted the notify-already-running branch January 2, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dehanj dehanj dehanj approved these changes

@mchack-work mchack-work Awaiting requested review from mchack-work

Assignees

@mchack-work mchack-work

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quite @dehanj @mchack-work