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

x/crypto/ssh/terminal: move all APIs to x/term and deprecate #31044

Closed
tandr opened this issue Mar 26, 2019 · 8 comments
Closed

x/crypto/ssh/terminal: move all APIs to x/term and deprecate #31044

tandr opened this issue Mar 26, 2019 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
Proposals (old)
Milestone
Unreleased

Comments

@tandr
Copy link

tandr commented Mar 26, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.1 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
not important

What did you do?

import (
	"os"
	"golang.org/x/crypto/ssh/terminal"
)

// abridged version
func checkIsTerminal(fd *osFile) bool {
	if fd != nil {
		return terminal.IsTerminal(int(fd.Fd()))
	}
	return false
}

What did you expect to see?

no "crypto" or "ssh" wording in import directive - there is nothing is going on what is cryptography-related

What did you see instead?

Since it has "crypto" part in a path, we have to explain to our legal department what do we do here, why do we need it, and how it affects whole solution software export restrictions.

It would be nice to have non-crypto and non-password related parts to be moved into library that has nothing to do with "crypto" or "ssh", as it definitely has more utility than just for ssh-related work.
@gopherbot gopherbot added this to the Unreleased milestone Mar 26, 2019
@dmitshur
Copy link
Contributor

We've created golang.org/x/term exactly for this purpose. But the move hasn't been done yet.

This is related to issue #13104.

/cc @matloob @bradfitz

@tandr tandr mentioned this issue Mar 26, 2019
Closed
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 3, 2019
@FiloSottile
Copy link
Contributor

I think we should move all exposed APIs over from x/crypto/ssh/terminal to x/term (using type aliases as needed) and mark x/crypto/ssh/terminal frozen and deprecated. It indeed makes no sense in x/crypto and that's what x/term is for.

@FiloSottile FiloSottile changed the title x/crypto: extract non-crypto parts from x/crypto/ssh/terminal into separate library proposal: x/crypto/ssh/terminal: move all APIs to x/term and deprecate Feb 4, 2020
@FiloSottile FiloSottile removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 4, 2020
@FiloSottile FiloSottile modified the milestones: Unreleased, Proposal Feb 4, 2020
@FiloSottile FiloSottile mentioned this issue Feb 4, 2020
Closed
@rsc
Copy link
Contributor

rsc commented Feb 5, 2020

It would be nice if we had a maintainer for any of this code.
Overall I guess it is OK if we just move that code wholesale into x/term.
I'm not really sure about the API, but it would at least clean things up
not to have it in crypto.

@rsc rsc added this to Active in Proposals (old) Feb 5, 2020
@rsc rsc mentioned this issue Feb 5, 2020
Open
@rsc
Copy link
Contributor

rsc commented Feb 12, 2020

This seems OK. We can't delete the old code but we can deprecate it.
I've seen no objections, so this seems like a likely accept.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Feb 12, 2020
@tandr
Copy link
Author

tandr commented Feb 21, 2020

@rsc -- what does "maintainer" role implies, sorry?

@ianlancetaylor
Copy link
Contributor

A maintainer is someone with experience with the code who is willing to take responsibility for reviewing contributions and fixing issues related to the code.

@rsc
Copy link
Contributor

rsc commented Feb 26, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Feb 26, 2020
@rsc rsc modified the milestones: Proposal, Unreleased Feb 26, 2020
@rsc rsc changed the title proposal: x/crypto/ssh/terminal: move all APIs to x/term and deprecate x/crypto/ssh/terminal: move all APIs to x/term and deprecate Feb 26, 2020
@FiloSottile FiloSottile added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 26, 2020
@FiloSottile FiloSottile self-assigned this Feb 26, 2020
@johningve johningve mentioned this issue Jul 3, 2020
Merged
@gopherbot
Copy link

Change https://golang.org/cl/258003 mentions this issue: ssh/terminal: replace with a golang.org/x/term wrapper

This was referenced Nov 23, 2020
Merged
Merged
Merged
dmitshur added a commit to shurcooL/markdownfmt that referenced this issue Dec 25, 2020
@dmitshur
switch to using golang.org/x/term
77d6907 
For golang/go#31044.
johejo added a commit to johejo/fzf that referenced this issue Mar 18, 2021
@johejo
replace golang.org/x/crypto/ssh/terminal with golang.org/x/term
6ef996f 
See golang/go#31044
@johejo johejo mentioned this issue Mar 18, 2021
Merged
johejo added a commit to johejo/fzf that referenced this issue Mar 18, 2021
@johejo
replace golang.org/x/crypto/ssh/terminal with golang.org/x/term
f6e6abc 
See golang/go#31044
junegunn pushed a commit to junegunn/fzf that referenced this issue Mar 20, 2021
@johejo
Replace golang.org/x/crypto/ssh/terminal with golang.org/x/term (#2395)
2e353ae 
See golang/go#31044
johejo added a commit to johejo/prompter that referenced this issue Mar 20, 2021
@johejo
Use golang.org/x/term instead of golang.org/x/crypto/ssh/terminal
b27aee8 
See golang/go#31044
@johejo johejo mentioned this issue Mar 20, 2021
Merged
johejo added a commit to johejo/sqldef that referenced this issue Apr 8, 2021
@johejo
Migrate to golang.org/x/term
b88970b 
See golang/go#31044
@johejo johejo mentioned this issue Apr 8, 2021
Merged
kralicky pushed a commit to kralicky/fzf that referenced this issue Jun 23, 2021
@johejo @kralicky
Replace golang.org/x/crypto/ssh/terminal with golang.org/x/term (june…
59fa9cc 
…gunn#2395)

See golang/go#31044
@tri-adam tri-adam mentioned this issue Aug 31, 2021
Closed
@golang golang locked and limited conversation to collaborators Nov 16, 2021
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@FiloSottile
ssh/terminal: replace with a golang.org/x/term wrapper
99f46ff 
The package moved to x/term in CL 258001.

Fixes golang/go#31044

Change-Id: I9d39bfb6f54f09de60e2669fb0d939968af79b40
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/258003
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@FiloSottile
ssh/terminal: replace with a golang.org/x/term wrapper
a69658c 
The package moved to x/term in CL 258001.

Fixes golang/go#31044

Change-Id: I9d39bfb6f54f09de60e2669fb0d939968af79b40
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/258003
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
No open projects
Proposals (old)
Accepted
Milestone
Unreleased
Development

No branches or pull requests
7 participants
@rsc @andybons @FiloSottile @dmitshur @ianlancetaylor @tandr @gopherbot