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

os: add ReadDir, ReadFile, WriteFile, CreateTemp, MkdirTemp & deprecate io/ioutil #42026

Closed
rsc opened this issue Oct 16, 2020 · 23 comments
Closed

os: add ReadDir, ReadFile, WriteFile, CreateTemp, MkdirTemp & deprecate io/ioutil #42026

rsc opened this issue Oct 16, 2020 · 23 comments

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Oct 16, 2020

io/ioutil, like most things with util in the name, has turned out to be a poorly defined and hard to understand collection of things.
As part of #40025, we migrated Discard, NopCloser, and ReadAll to package io.
(They remain in io/ioutil for compatibility, of course, but new code should prefer the ones in io.)
What's left is a few OS file system helpers: ReadDir, ReadFile, TempDir, TempFile, and WriteFile.
I propose we migrate all of these to package os, with a few adjustments.
(Again, they would remain here for compatibility, but new code would prefer the ones in os.)

At that point, io/ioutil would become entirely deprecated.

The signatures would be:

package os

// ReadDir reads the named directory,
// returning all its directory entries sorted by filename.
func ReadDir(name string) ([]DirEntry, error)

// ReadFile reads the named file and returns the contents.
// A successful call returns err == nil, not err == EOF.
// Because ReadFile reads the whole file, it does not treat an EOF from Read
// as an error to be reported.
func ReadFile(name string) ([]byte, error)

// WriteFile writes data to the named file, creating it if necessary.
// If the file does not exist, WriteFile creates it with permissions perm (before umask);
// otherwise WriteFile truncates it before writing, without changing permissions.
func WriteFile(name string, data []byte, perm fs.FileMode) error

// MkdirTemp creates a new temporary directory in the directory dir
// and returns the pathname of the new directory.
// The new directory's name is generated by adding a random string to the end of pattern.
// If pattern includes a "*", the random string replaces the last "*" instead.
// If dir is the empty string, TempFile uses the default directory for temporary files, as returned by TempDir.
// Multiple programs or goroutines calling MkdirTemp simultaneously will not choose the same directory.
// It is the caller's responsibility to remove the directory when it is no longer needed.
func MkdirTemp(dir, pattern string) (string, error)

// CreateTemp creates a new temporary file in the directory dir,
// opens the file for reading and writing, and returns the resulting file.
// The filename is generated by taking pattern and adding a random string to the end.
// If pattern includes a "*", the random string replaces the last "*".
// If dir is the empty string, TempFile uses the default directory for temporary files, as returned by TempDir.
// Multiple programs or goroutines calling CreateTemp simultaneously will not choose the same file.
// The caller can use the file's Name method to find the pathname of the file.
// It is the caller's responsibility to remove the file when it is no longer needed.
func CreateTemp(dir, pattern string) (*File, error)

Notes:

  • os.ReadDir returns []DirEntry, in contrast to ioutil.ReadDir's []FileInfo.
    (Providing a helper that returns []DirEntry is one of the primary motivations for this change.)

  • os.ReadFile is identical to ioutil.ReadFile.

  • os.WriteFile is identical to ioutil.WriteFile.
    I looked into whether the permission bits should be dropped, but Go code in the wild uses a variety of popular settings (for example: 0666, 0644, 0600, 0700, 0777).

  • os.MkdirTemp is identical to ioutil.TempDir.
    It cannot be named os.TempDir because that already exists and returns the default temporary directory.
    MkdirTemp seems like it makes clear that the directory is being created and will appear next to Mkdir in docs.
    I looked into whether the directory argument should be dropped, but about 20% of calls in the wild don't use the empty string.

  • os.CreateTemp is identical to ioutil.TempFile.
    It could be named os.TempFile, but calling it os.CreateTemp is consistent with os.MkdirTemp, makes clear that the file is being created (using TempFile for this would be a function surprisingly different from TempDir), and will appear next to Create in docs.
    I looked into whether the directory argument should be dropped, but about 25% of calls in the wild don't use the empty string.

@rsc rsc added this to the Proposal milestone Oct 16, 2020
@bcmills
Copy link
Member

@bcmills bcmills commented Oct 17, 2020

Would it make sense to put ReadDir, ReadFile, and WriteFile in io/fs instead of os?
It sees like they could plausibly work with any filesystem, not just the os one.

I would argue that they fit naturally with the new Walk API, wherever that ends up: ReadDir, ReadFile, WriteFile, and Walk all share the property that they implement a high-level operation in terms of existing lower-level operations.

@bcmills
Copy link
Member

@bcmills bcmills commented Oct 17, 2020

(MkdirTemp and CreateTemp seem like they belong in os, though: the location of the temporary directory is OS-specific, and CreateTemp, at least, should return a *os.File rather than an abstract fs.File.)

@bcmills
Copy link
Member

@bcmills bcmills commented Oct 17, 2020

Actually, even MkdirTemp and CreateTemp could belong in io/fs, if we define an FS extension interface for a method that returns a default directory for temporary files.

@rsc
Copy link
Contributor Author

@rsc rsc commented Oct 20, 2020

@bcmills, io/fs already has ReadDir and ReadFile. It would be fine to add CreateTemp and MkdirTemp if/when io/fs adds any writing operations.

But it probably wouldn't make sense to put them only in io/fs. Reading from the operating system's file systems is insanely common. For example, even if we were starting over we would certainly keep os.Open(name) rather than tell people to use fs.Open(os.Dir("/"), name) or os.Dir("/").Open(name) (neither of which would be correct on Windows anyway).

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Oct 28, 2020
@rsc rsc moved this from Incoming to Active in Proposals Oct 28, 2020
@rsc
Copy link
Contributor Author

@rsc rsc commented Oct 29, 2020

Accepting this issue will obsolete #19660, which proposes renaming ioutil.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 29, 2020

Change https://golang.org/cl/266365 mentions this issue: all: update to use os.ReadFile, os.WriteFile, os.CreateTemp, os.MkdirTemp

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 29, 2020

Change https://golang.org/cl/266364 mentions this issue: os: add ReadFile, WriteFile, CreateTemp (was TempFile), MkdirTemp (was TempDir) from io/ioutil

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 29, 2020

Change https://golang.org/cl/266366 mentions this issue: all: update to use os.ReadDir where appropriate

@rsc
Copy link
Contributor Author

@rsc rsc commented Oct 29, 2020

I've posted the CLs that would implement this change in case anyone wants to see what the diffs look like.
I am particularly happy that in the two CLs converting the tree to use the new locations, 217 "io/ioutil" imports were removed but only 51 "os" imports were added, meaning that in ~75% of cases the code was already importing "os" and is simplified by not having to juggle ioutil as well.

@rsc
Copy link
Contributor Author

@rsc rsc commented Nov 4, 2020

Based on the discussion, this seems like a likely accept.

@rsc rsc moved this from Active to Likely Accept in Proposals Nov 4, 2020
@carlmjohnson
Copy link
Contributor

@carlmjohnson carlmjohnson commented Nov 5, 2020

Getting rid of ioutil is a good conceptual clean up. I know when I first learned Go, I was confused about what was in io vs. ioutil vs. bufio. io itself is a little tricky to understand, but with the cleanup, the explanation is pretty clear. I can imagine writing a blog post for beginners along the lines of:

Package io defines different kinds of "virtual files" either for reading or writing with special features like seeking and closing. io/fs extends the virtual file into a virtual filesystem. In most case, you won't work with io.Readers and io.Writers directly, but either stream them with package bufio or write them to memory with bytes.Buffer or strings.Builder.

The only thing that seems odd given that description is that FileInfo and FileMode are in io/fs instead of io itself. On the one hand, they're part of a "virtual file" so you'd think they'd be in io. OTOH, they only make sense as part of a "virtual filesystem" in which case they should be in io/fs…

@rsc
Copy link
Contributor Author

@rsc rsc commented Nov 11, 2020

Instead of "virtual files" say "virtual data streams" and you're all set!

@rsc
Copy link
Contributor Author

@rsc rsc commented Nov 11, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals Nov 11, 2020
@rsc rsc changed the title proposal: os: add ReadDir, ReadFile, WriteFile, CreateTemp, MkdirTemp & deprecate io/ioutil os: add ReadDir, ReadFile, WriteFile, CreateTemp, MkdirTemp & deprecate io/ioutil Nov 11, 2020
@rsc rsc removed this from the Proposal milestone Nov 11, 2020
@rsc rsc added this to the Backlog milestone Nov 11, 2020
gopherbot pushed a commit that referenced this issue Dec 2, 2020
…s TempDir) from io/ioutil

io/ioutil was a poorly defined collection of helpers.
Proposal #40025 moved out the generic I/O helpers to io.
This CL for proposal #42026 moves the OS-specific helpers to os,
making the entire io/ioutil package deprecated.

For #42026.

Change-Id: I018bcb2115ef2ff1bc7ca36a9247eda429af21ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/266364
Trust: Russ Cox <rsc@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
gopherbot pushed a commit that referenced this issue Dec 9, 2020
…Temp

As part of #42026, these helpers from io/ioutil were moved to os.
(ioutil.TempFile and TempDir became os.CreateTemp and MkdirTemp.)

Update the Go tree to use the preferred names.

As usual, code compiled with the Go 1.4 bootstrap toolchain
and code vendored from other sources is excluded.

ReadDir changes are in a separate CL, because they are not a
simple search and replace.

For #42026.

Change-Id: If318df0216d57e95ea0c4093b89f65e5b0ababb3
Reviewed-on: https://go-review.googlesource.com/c/go/+/266365
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot gopherbot closed this in f1980ef Dec 9, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 17, 2020

Change https://golang.org/cl/279072 mentions this issue: os: remove dependency on strings package

gopherbot pushed a commit that referenced this issue Dec 18, 2020
Historically the os package has not imported the strings package.
That was enforced by go/build.TestDependencies, but that test
was accidentally broken (#43249). A dependency of os on strings
was accidentally added by CL 266364; remove it.

For #42026
For #43249

Change-Id: If932308f30561fdcc5c608d7563e849c0d2870d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/279072
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
@go101
Copy link

@go101 go101 commented Dec 21, 2020

Should "go fix" support the migration for old uses to the new ones?

@bcmills
Copy link
Member

@bcmills bcmills commented Dec 21, 2020

@go101, that would fall out naturally from #32816.

dlsniper added a commit to dlsniper/go that referenced this issue Jan 20, 2021
All implementations are now part of io or os packages, per golang#42026.
Flag all implementations in ioutil, and the package itself, with
the Deprecated marker so that tooling can pick it up and interpret
it accordingly.

Updates golang#42026
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 20, 2021

Change https://golang.org/cl/284777 mentions this issue: io/ioutil: flag package and functions with the Deprecated marker

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 22, 2021

Change https://golang.org/cl/285376 mentions this issue: os: correct name in MkdirTemp doc comment

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 22, 2021

Change https://golang.org/cl/285377 mentions this issue: doc/go1.16: mention deprecation of io/ioutil

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 22, 2021

Change https://golang.org/cl/285378 mentions this issue: io/ioutil: forward TempFile and TempDir to os package

@dmitshur dmitshur removed this from the Backlog milestone Jan 25, 2021
@dmitshur dmitshur added this to the Go1.16 milestone Jan 25, 2021
gopherbot pushed a commit that referenced this issue Jan 25, 2021
For #40025
For #40700
For #42026

Change-Id: Ib51b5e1398c4eb811506df21e3bd56dd84bd1f7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/285377
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
gopherbot pushed a commit that referenced this issue Jan 26, 2021
For #42026

Change-Id: I51e3ce9d3a4729cfac44bd3ff3f3ec80dcd5abb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/285376
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 28, 2021

Change https://golang.org/cl/287613 mentions this issue: cmd/go: revert TestScript/build_trimpath to use ioutil.ReadFile

gopherbot pushed a commit that referenced this issue Jan 28, 2021
This call was changed to os.ReadFile in CL 266365, but the test also
builds that source file using gccgo if present, and released versions
of gccgo do not yet support ioutil.ReadFile.

Manually tested with gccgo gccgo 10.2.1 (see #35786).

Fixes #43974.
Updates #42026.

Change-Id: Ic4ca0848d3ca324e2ab10fd14ad867f21e0898e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/287613
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Feb 24, 2021
For #42026
Fixes #44311

Change-Id: I3dabcf902d155f95800b4adf1d7578906a194ce6
Reviewed-on: https://go-review.googlesource.com/c/go/+/285378
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
@Levis-Code
Copy link

@Levis-Code Levis-Code commented Aug 16, 2021

Por favor alguien que sepa espanol y me explique esto

[oslay@oslay-garuda ~]$ sudo go get layeh.com/asar
go get: module layeh.com/asar: Get "https://proxy.golang.org/layeh.com/asar/@v/list": read tcp 192.168.1.176:40832->142.250.189.145:443: read: connection reset by peer

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 16, 2021

@Levis-Code Si tiene preguntas sobre Go, consulte https://golang.org/wiki/Questions .

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
9 participants