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

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

Open
tandr opened this issue Mar 26, 2019 · 7 comments
Open

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

tandr opened this issue Mar 26, 2019 · 7 comments

Comments

@tandr
Copy link

@tandr 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

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Mar 26, 2019

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

@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Feb 4, 2020

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
@gopherbot gopherbot added the Proposal label Feb 4, 2020
@FiloSottile FiloSottile modified the milestones: Unreleased, Proposal Feb 4, 2020
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc 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

This comment has been minimized.

Copy link
Contributor

@rsc 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 Feb 12, 2020
@tandr

This comment has been minimized.

Copy link
Author

@tandr tandr commented Feb 21, 2020

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

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 21, 2020

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

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Feb 26, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals 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 self-assigned this Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Accepted
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.