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

all: remove existing uses of io/ioutil package #45557

Open
perillo opened this issue Apr 14, 2021 · 8 comments
Open

all: remove existing uses of io/ioutil package #45557

perillo opened this issue Apr 14, 2021 · 8 comments
Labels
Milestone

Comments

@perillo
Copy link
Contributor

@perillo perillo commented Apr 14, 2021

The io/ioutil package is still used in go/build and net/http.

It is also widely used in the cmd module. I'm not sure if there was a reason why it was removed in std but not in cmd.

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 14, 2021

In Context.readDir there is a TODO note to "use os.ReadDir". However os.ReadDir returns fs.DirEntry so it is not possible to used it. Should the TODO note be removed?

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 14, 2021

Change https://golang.org/cl/310029 mentions this issue: net/http: replace ioutil.NopCloser with io.NopCloser

@tklauser
Copy link
Member

@tklauser tklauser commented Apr 14, 2021

It is also widely used in the cmd module. I'm not sure if there was a reason why it was removed in std but not in cmd.

Note that cmd/dist and (I think) some parts of cmd/compile and cmd/link need to remain buildable with Go 1.4 for bootstraping. Thus, these cannot use features newer than Go 1.4. Also see src/cmd/dist/README.

However, #44505 proposes to adopt Go 1.16 as the bootstrap toolchain for Go 1.18. That proposal was accepted, so we'll probably be able to replace io/ioutil there as well for Go 1.18.

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 14, 2021

@tklauser, right thanks. I always forgot that go is bootstrapped from go1.4. Fortunately I avoided touching the tests in the cmd module.

@mknyszek mknyszek changed the title std: remove existing use of io/ioutil package std: remove existing uses of io/ioutil package Apr 14, 2021
@mknyszek mknyszek added this to the Backlog milestone Apr 14, 2021
@mknyszek
Copy link
Contributor

@mknyszek mknyszek commented Apr 14, 2021

Since we can't really do this in large parts of the codebase for now, I suppose this can just be a tracking issue?

CC @rsc who deprecated io/ioutil (I'm honestly not sure who to CC on this, though).

@perillo
Copy link
Contributor Author

@perillo perillo commented Apr 14, 2021

@mknyszek: in the issue name I specified these changes to only apply to the std module, so it is ok. In future I will open another issue about the cmd module.

@mknyszek mknyszek changed the title std: remove existing uses of io/ioutil package all: remove existing uses of io/ioutil package Apr 14, 2021
@rsc
Copy link
Contributor

@rsc rsc commented Jun 3, 2021

It's fine to leave this issue open but let's not do this work by hand. I intend to automate it as part of a more general "go fix" when the time comes.

@perillo
Copy link
Contributor Author

@perillo perillo commented Jun 3, 2021

Ok, thanks.

Currently, in the std module all imports of the io/ioutil package have been replaced, excluding go/build since Context.readDir calls ioutil.ReadDir (there is a TODO note that is probably wrong, since Context.readDir should call File.Readdir and not os.ReadDir, so the change should me done manually).

io/ioutil is also imported in the net/http package, in code generated from golang.org/x/net/http2, and in some vendored packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants