Skip to content

x/tools/cmd/auth: tag and delete #70872

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

Open
samthanawalla opened this issue Dec 16, 2024 · 25 comments
Open

x/tools/cmd/auth: tag and delete #70872

samthanawalla opened this issue Dec 16, 2024 · 25 comments

Comments

@samthanawalla
Copy link
Contributor

samthanawalla commented Dec 16, 2024

Proposal Details

The x/tools/cmd/auth directory is obsolete. We've integrated its GOAUTH implementation directly into src/cmd/go/internal/auth as part of issue #26232. This means we no longer need to maintain the separate reference implementation in x/tools/cmd/auth.

Therefore, let's delete x/tools/cmd/auth

@adonovan
Copy link
Member

BTW, the process for deleting a package is to create a new module in its subdirectory, release it by tagging a version, and then delete it. That way any project that imports it will automatically depend on the deleted module (after upgrading) and will continue to build. See for example #59676.

@adonovan adonovan moved this to Incoming in Proposals Dec 16, 2024
@shashank-priyadarshi
Copy link

@samthanawalla can I please take this up?

@samthanawalla
Copy link
Contributor Author

@samthanawalla can I please take this up?

That would be great! However the proposal needs to get accepted first

@rsc rsc changed the title proposal: x/tools/cmd/auth: cleanup old code proposal: x/tools/cmd/auth: tag and delete Feb 5, 2025
@rsc
Copy link
Contributor

rsc commented Feb 5, 2025

It looks like netrcauth and gitauth are integrated into the go command but not cookieauth. Should we leave cookieauth around or provide it somewhere else?

@rsc
Copy link
Contributor

rsc commented Feb 5, 2025

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals Feb 5, 2025
@samthanawalla
Copy link
Contributor Author

I think it makes sense to move cookieauth somewhere else so as to not confuse people but I am not sure where.

@aclements
Copy link
Member

Why was cookieauth not added to cmd/go's built-in support the way gitauth and netrcauth were? If it were also built in, then we could tag and delete the whole tree.

@aclements
Copy link
Member

@samthanawalla , ping on my above question. Thanks!

@samthanawalla
Copy link
Contributor Author

The accepted proposal did not include cookieauth as a built-in command.

@aclements
Copy link
Member

Thanks.

I'm having a lot of trouble reconstructing the history here. I see on Feb 7, 2019, @bcmills proposed what eventually became the GOAUTH mechanism. The next day, he created the three reference implementations, x/tools/cmd/auth/{gitauth,netrcauth,cookieauth}. His proposal doesn't include a cookieauth setting for GOAUTH, though his rationale mentions cookies.txt and it's in his reference implementations, which makes me wonder if this was an unintentional omission. There doesn't seem to be any discussion one way or the other about cookieauth in #26232 other than one early mention that cookies.txt is preferable to netrc.

Then it looks like GOAUTH support (including git, netrc, and custom commands) was released in Go 1.24.

None of this really explains why cookieauth has a reference implementation, but wasn't part of GOAUTH. Independent of the question of tagging and deleting x/tools/cmd/auth, does it make sense to add built-in cookies.txt support to GOAUTH? It seems like it does, but I'm not an expert on this. If the answer is "yes", then we should just do that and then there's no issue with tagging and deleting x/tools/cmd/auth wholesale.

@samthanawalla
Copy link
Contributor Author

Thanks for looking into that. I missed the mention of cookies.txt in the proposal.

At this point, we can either (1) proceed with implementing cookieauth in 1.25 or (2) wait for increased demand of cookieauth.

I would say number 2 makes sense as there are probably other priorities for the Go Command in the backlog.

@aclements
Copy link
Member

Thanks.

I think we can move forward with tagging and deleting this. go install golang.org/x/tools/cmd/auth/cookieauth@latest would continue to work because that will resolve to the latest tag of that specific package.

If it turns out there's enough demand for cookieauth, we can consider adding built-in support.

@aclements
Copy link
Member

Based on the discussion above, this proposal seems like a likely accept.
— aclements for the proposal review group

The proposal is to tag and delete the golang.org/x/tools/cmd/auth packages.

With the exception of cookieauth, these have been integrated into the go command, so the external helpers are not needed. People can continue to use the last version of the external cookieauth helper before it was deleted, and if there's demand we can revisit adding built-in support for cookieauth to cmd/go.

@aclements aclements moved this from Active to Likely Accept in Proposals Mar 26, 2025
@aclements
Copy link
Member

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— aclements for the proposal review group

The proposal is to tag and delete the golang.org/x/tools/cmd/auth packages.

With the exception of cookieauth, these have been integrated into the go command, so the external helpers are not needed. People can continue to use the last version of the external cookieauth helper before it was deleted, and if there's demand we can revisit adding built-in support for cookieauth to cmd/go.

@aclements aclements moved this from Likely Accept to Accepted in Proposals Apr 2, 2025
@aclements aclements changed the title proposal: x/tools/cmd/auth: tag and delete x/tools/cmd/auth: tag and delete Apr 2, 2025
@aclements aclements modified the milestones: Proposal, Backlog Apr 2, 2025
@shashank-priyadarshi
Copy link

@samthanawalla glad to take it off your plate!

@shashank-priyadarshi
Copy link

Hi @samthanawalla , is there a specific convention to follow for naming tags for obsolete modules? If any such convention/documentation exists, could you please share it here!
If not, should I go ahead with this: cmd/auth/latest-deprecated , as it is in line with tags for other deprecated packages listed below:

cmd/cover/v0.1.0-deprecated
cmd/getgo/v0.1.0-deprecated
cmd/gorename/v0.1.0-deprecated
cmd/guru/v0.1.0-deprecated
cmd/guru/v0.1.1-deprecated
go/pointer/v0.1.0-deprecated
go/vcs/v0.1.0-deprecated

@shashank-priyadarshi
Copy link

@samthanawalla ping on the above question!

@samthanawalla
Copy link
Contributor Author

v0.1.0-deprecated should be fine

@shashank-priyadarshi
Copy link

shashank-priyadarshi commented Apr 24, 2025

Hi @samthanawalla , submitted change on gerrit to tag and release auth module.

@adonovan
Copy link
Member

Hi @samthanawalla , submitted change on gerrit to tag and release auth module.

Which CL was this? Creating a tag usually requires a temporary escalation of privilege through a tool that requires approval of two Googlers.

@shashank-priyadarshi
Copy link

shashank-priyadarshi commented Apr 24, 2025

This has not yet been approved @adonovan . Added reviewers are @samthanawalla & @matloob

@adonovan
Copy link
Member

This has not yet been approved

What is "this"? (This proposal? It has been approved. A CL? Which one?)

@shashank-priyadarshi
Copy link

It is a change that I have submitted through Gerrit. Here's the link: https://go-review.googlesource.com/c/tools/+/666975
Please let me know if changes are required.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/669035 mentions this issue: cmd/auth: tag and delete deprecated auth module

@matloob
Copy link
Contributor

matloob commented May 6, 2025

@shashank-priyadarshi Your change deletes the auth module, but we need to tag the module first, as @adonovan mentioned above.

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

No branches or pull requests

7 participants