From 42070272a96353f503c0a7dca1816f07e2c300e6 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Tue, 15 Oct 2019 20:37:16 +0200 Subject: [PATCH 01/14] design: add proposal doc for 33974 Updates golang/go#33974 --- design/33974-add-public-lockedfile-pkg.md | 140 ++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 design/33974-add-public-lockedfile-pkg.md diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md new file mode 100644 index 00000000..555e5404 --- /dev/null +++ b/design/33974-add-public-lockedfile-pkg.md @@ -0,0 +1,140 @@ +# Proposal: make the internal [lockedfile](https://godoc.org/github.com/golang/go/src/cmd/go/internal/lockedfile/) package public + +Author(s): [Adrien Delorme] + +Last updated: 2019-10-15 + +Discussion at https://golang.org/issue/33974. + +## Abstract + +Move already existing code residing golang/go/src/cmd/go/internal/lockedfile to +`x/exp`. At `x/exp/lockedfile`. + +## Background + +A few Golang open source projects are implementing file locking mechanisms +( https://github.com/gofrs/flock, https://github.com/MichaelS11/go-file-lock, +https://github.com/juju/fslock ) but they do not seem to be maintained anymore, +issues are ignored and they seem untrusted. +As a result some major projects are doing their own version of it; ex: +[terraform](https://github.com/hashicorp/terraform/blob/1ff9a540202b8c36e33db950374bbb4495737d8f/states/statemgr/filesystem_lock_unix.go), +[boltdb](https://github.com/boltdb/bolt/search?q=flock&unscoped_q=flock). +After some researches it seemed to us that the already existing and maintained +[lockedfile](https://godoc.org/github.com/golang/go/src/cmd/go/internal/lockedfile/) +package is the best 'open source' version. + +File locking seems complicated and low level enough to be more closely +maintained by the go community ( and/or team ? ). +We think it would benefit the mass to make such a package public: since it's +already being part of the go code and therefore being maintained; it should be +made public. + +## Proposal + +We propose to copy the golang/go/src/cmd/go/internal/lockedfile to `x/exp`. To +make it public. Not changing any of the named types for now. + +Exported names and comments as can be currently found in +[07b4abd](https://github.com/golang/go/tree/07b4abd62e450f19c47266b3a526df49c01ba425/src/cmd/go/internal/lockedfile): + +``` +// Package lockedfile creates and manipulates files whose contents should only +// change atomically. +package lockedfile + +//Read opens the named file with a read-lock and returns its contents. +func Read(name string) ([]byte, error) + +// Write opens the named file (creating it with the given permissions if +// needed), then write-locks it and overwrites it with the given content. +func Write(name string, content io.Reader, perm os.FileMode) (err error) + +// A File is a locked *os.File. +// +// Closing the file releases the lock. +// +// If the program exits while a file is locked, the operating system releases +// the lock but may not do so promptly: callers must ensure that all locked +// files are closed before exiting. +type File struct { + // contains filtered or unexported fields +} + +// Create is like os.Create, but returns a write-locked file. +* func Create(name string) (*File, error) + +// Edit creates the named file with mode 0666 (before umask), +// but does not truncate existing contents. +// +// If Edit succeeds, methods on the returned File can be used for I/O. +// The associated file descriptor has mode O_RDWR and the file is write-locked. +* func Edit(name string) (*File, error) + +// Open is like os.Open, but returns a read-locked file. +* func Open(name string) (*File, error) + +// OpenFile is like os.OpenFile, but returns a locked file. +// If flag includes os.O_WRONLY or os.O_RDWR, the file is write-locked; +// otherwise, it is read-locked. +* func OpenFile(name string, flag int, perm os.FileMode) (*File, error) + +// Close unlocks and closes the underlying file. +// +// Close may be called multiple times; all calls after the first will return a +// non-nil error. +* func (f *File) Close() error + +// A Mutex provides mutual exclusion within and across processes by locking a +// well-known file. Such a file generally guards some other part of the +// filesystem: for example, a Mutex file in a directory might guard access to +// the entire tree rooted in that directory. +// +// Mutex does not implement sync.Locker: unlike a sync.Mutex, a lockedfile.Mutex +// can fail to lock (e.g. if there is a permission error in the filesystem). +// +// Like a sync.Mutex, a Mutex may be included as a field of a larger struct but +// must not be copied after first use. The Path field must be set before first +// use and must not be change thereafter. +type Mutex struct { + Path string // The path to the well-known lock file. Must be non-empty. + // contains filtered or unexported fields +} + +// MutexAt returns a new Mutex with Path set to the given non-empty path. +* func MutexAt(path string) *Mutex + +// Lock attempts to lock the Mutex. +// +// If successful, Lock returns a non-nil unlock function: it is provided as a +// return-value instead of a separate method to remind the caller to check the +// accompanying error. (See https://golang.org/issue/20803.) +* func (mu *Mutex) Lock() (unlock func(), err error) + +// String returns a string containing the path of the mutex. +* func (mu *Mutex) String() string +``` + +## Rationale + +The golang/go/src/cmd/go/internal/lockedfile already exists but has untrusted & +unmaintained alternatives. + +* Making this package public will make it more used. A tiny surge of issues + might come in the beginning; at the benefits of everyone. ( Unless it's + bug free !! ). + +* There exists a https://godoc.org/github.com/rogpeppe/go-internal package that + exports a lot of internal packages from the go repo. But if go-internal + became wildly popular; in order to have a bug fixed or a feature introduced + in; a user would still need to open a PR on the go repo; then the author of + go-internal would need to update the package. + +## Compatibility + +There are no compatibility issues. Since this will be a code addition. + +## Implementation + +Adrien Delorme plans to do copy the internal/lockedfile package from cmd/go to +`x/exp`. \ No newline at end of file From dfd8558017bcfe2d438619b1ad5b50fc5fb3cf26 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 10:58:11 +0200 Subject: [PATCH 02/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 50 ++++++++++++++--------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index 555e5404..5138dd13 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -13,19 +13,29 @@ Move already existing code residing golang/go/src/cmd/go/internal/lockedfile to ## Background -A few Golang open source projects are implementing file locking mechanisms -( https://github.com/gofrs/flock, https://github.com/MichaelS11/go-file-lock, -https://github.com/juju/fslock ) but they do not seem to be maintained anymore, -issues are ignored and they seem untrusted. -As a result some major projects are doing their own version of it; ex: +A few open source Go projects are implementing file locking mechanisms but they +do not seem to be maintained anymore: + https://github.com/gofrs/flock : This repo has accepted PRs as recently as + this March, so this implementation may be maintained — but it is not (yet?) + portable to as many platforms as the implementation in the Go project, and + we could argue that our `lockedfile` package API is more ergonomic anyway. + https://github.com/juju/fslock : Note that this implementation is both + unmaintained and LGPL-licensed, so even folks who would like to use it might + not be able to. Also not that this repo [was selected for removal in + 2017](https://github.com/juju/fslock/issues/4) + + +As a result some major projects are doing +their own version of it; ex: [terraform](https://github.com/hashicorp/terraform/blob/1ff9a540202b8c36e33db950374bbb4495737d8f/states/statemgr/filesystem_lock_unix.go), -[boltdb](https://github.com/boltdb/bolt/search?q=flock&unscoped_q=flock). -After some researches it seemed to us that the already existing and maintained +[boltdb](https://github.com/boltdb/bolt/search?q=flock&unscoped_q=flock). After +some researches it seemed to us that the already existing and maintained [lockedfile](https://godoc.org/github.com/golang/go/src/cmd/go/internal/lockedfile/) package is the best 'open source' version. -File locking seems complicated and low level enough to be more closely -maintained by the go community ( and/or team ? ). +File-locking interacts pretty deeply with the `os` package and the system call +library in `x/sys`, so it makes sense for (a subset of) the same owners to +consider the evolution of those packages together. We think it would benefit the mass to make such a package public: since it's already being part of the go code and therefore being maintained; it should be made public. @@ -43,7 +53,7 @@ Exported names and comments as can be currently found in // change atomically. package lockedfile -//Read opens the named file with a read-lock and returns its contents. +// Read opens the named file with a read-lock and returns its contents. func Read(name string) ([]byte, error) // Write opens the named file (creating it with the given permissions if @@ -62,28 +72,28 @@ type File struct { } // Create is like os.Create, but returns a write-locked file. -* func Create(name string) (*File, error) +func Create(name string) (*File, error) // Edit creates the named file with mode 0666 (before umask), // but does not truncate existing contents. // // If Edit succeeds, methods on the returned File can be used for I/O. // The associated file descriptor has mode O_RDWR and the file is write-locked. -* func Edit(name string) (*File, error) +func Edit(name string) (*File, error) // Open is like os.Open, but returns a read-locked file. -* func Open(name string) (*File, error) +func Open(name string) (*File, error) // OpenFile is like os.OpenFile, but returns a locked file. // If flag includes os.O_WRONLY or os.O_RDWR, the file is write-locked; // otherwise, it is read-locked. -* func OpenFile(name string, flag int, perm os.FileMode) (*File, error) +func OpenFile(name string, flag int, perm os.FileMode) (*File, error) // Close unlocks and closes the underlying file. // // Close may be called multiple times; all calls after the first will return a // non-nil error. -* func (f *File) Close() error +func (f *File) Close() error // A Mutex provides mutual exclusion within and across processes by locking a // well-known file. Such a file generally guards some other part of the @@ -102,17 +112,17 @@ type Mutex struct { } // MutexAt returns a new Mutex with Path set to the given non-empty path. -* func MutexAt(path string) *Mutex +func MutexAt(path string) *Mutex // Lock attempts to lock the Mutex. // // If successful, Lock returns a non-nil unlock function: it is provided as a // return-value instead of a separate method to remind the caller to check the // accompanying error. (See https://golang.org/issue/20803.) -* func (mu *Mutex) Lock() (unlock func(), err error) +func (mu *Mutex) Lock() (unlock func(), err error) // String returns a string containing the path of the mutex. -* func (mu *Mutex) String() string +func (mu *Mutex) String() string ``` ## Rationale @@ -120,11 +130,11 @@ type Mutex struct { The golang/go/src/cmd/go/internal/lockedfile already exists but has untrusted & unmaintained alternatives. -* Making this package public will make it more used. A tiny surge of issues +Making this package public will make it more used. A tiny surge of issues might come in the beginning; at the benefits of everyone. ( Unless it's bug free !! ). -* There exists a https://godoc.org/github.com/rogpeppe/go-internal package that +There exists a https://godoc.org/github.com/rogpeppe/go-internal package that exports a lot of internal packages from the go repo. But if go-internal became wildly popular; in order to have a bug fixed or a feature introduced in; a user would still need to open a PR on the go repo; then the author of From 9f6c5ebdf430df9a8d108c8b5defec7c133037f3 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 11:01:19 +0200 Subject: [PATCH 03/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index 5138dd13..45f8d676 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -8,8 +8,8 @@ Discussion at https://golang.org/issue/33974. ## Abstract -Move already existing code residing golang/go/src/cmd/go/internal/lockedfile to -`x/exp`. At `x/exp/lockedfile`. +Move already existing code residing in +`golang/go/src/cmd/go/internal/lockedfile` to `x/exp`. At `x/exp/lockedfile`. ## Background From de50d4fb4ae0690381df5ff70e0b3941f36b6027 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 11:02:40 +0200 Subject: [PATCH 04/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index 45f8d676..af8f7cdc 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -53,13 +53,6 @@ Exported names and comments as can be currently found in // change atomically. package lockedfile -// Read opens the named file with a read-lock and returns its contents. -func Read(name string) ([]byte, error) - -// Write opens the named file (creating it with the given permissions if -// needed), then write-locks it and overwrites it with the given content. -func Write(name string, content io.Reader, perm os.FileMode) (err error) - // A File is a locked *os.File. // // Closing the file releases the lock. From 84472999ca24106570539a219c76bb9f1eb5115d Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 11:12:09 +0200 Subject: [PATCH 05/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index af8f7cdc..b96ae234 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -15,11 +15,11 @@ Move already existing code residing in A few open source Go projects are implementing file locking mechanisms but they do not seem to be maintained anymore: - https://github.com/gofrs/flock : This repo has accepted PRs as recently as +* https://github.com/gofrs/flock : This repo has accepted PRs as recently as this March, so this implementation may be maintained — but it is not (yet?) portable to as many platforms as the implementation in the Go project, and we could argue that our `lockedfile` package API is more ergonomic anyway. - https://github.com/juju/fslock : Note that this implementation is both +* https://github.com/juju/fslock : Note that this implementation is both unmaintained and LGPL-licensed, so even folks who would like to use it might not be able to. Also not that this repo [was selected for removal in 2017](https://github.com/juju/fslock/issues/4) @@ -123,11 +123,11 @@ func (mu *Mutex) String() string The golang/go/src/cmd/go/internal/lockedfile already exists but has untrusted & unmaintained alternatives. -Making this package public will make it more used. A tiny surge of issues - might come in the beginning; at the benefits of everyone. ( Unless it's - bug free !! ). +* Exporting this package public will make it more used. A tiny surge of issues + might come in the beginning; at the benefits of everyone. ( Unless it's bug + free !! ). -There exists a https://godoc.org/github.com/rogpeppe/go-internal package that +* There exists a https://godoc.org/github.com/rogpeppe/go-internal package that exports a lot of internal packages from the go repo. But if go-internal became wildly popular; in order to have a bug fixed or a feature introduced in; a user would still need to open a PR on the go repo; then the author of @@ -135,9 +135,13 @@ There exists a https://godoc.org/github.com/rogpeppe/go-internal package that ## Compatibility -There are no compatibility issues. Since this will be a code addition. +There are no compatibility issues. Since this will be a code addition but +Ideally we don't want to maintain two copies of this package going forward, and +we probably don't want to vendor `x/exp` into the `cmd` module. + +Perhaps that implies that this should go in the `x/sys` or `x/sync` repo instead? ## Implementation -Adrien Delorme plans to do copy the internal/lockedfile package from cmd/go to -`x/exp`. \ No newline at end of file +Adrien Delorme plans to do copy the exported types in the proposal section from + `cmd/go/internal/lockedfile` to `x/sync`. \ No newline at end of file From 2a5edb9e8c37b7953411ad41efb13af8e581e5c8 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 11:22:57 +0200 Subject: [PATCH 06/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index b96ae234..8c5bf163 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -61,10 +61,11 @@ package lockedfile // the lock but may not do so promptly: callers must ensure that all locked // files are closed before exiting. type File struct { - // contains filtered or unexported fields + // contains unexported fields } // Create is like os.Create, but returns a write-locked file. +// If the file already exists, it is truncated. func Create(name string) (*File, error) // Edit creates the named file with mode 0666 (before umask), @@ -78,8 +79,8 @@ func Edit(name string) (*File, error) func Open(name string) (*File, error) // OpenFile is like os.OpenFile, but returns a locked file. -// If flag includes os.O_WRONLY or os.O_RDWR, the file is write-locked; -// otherwise, it is read-locked. +// If flag implies edition (ie: os.O_TRUNC, os.O_WRONLY or os.O_RDWR), the file +// is write-locked; otherwise, it is read-locked. func OpenFile(name string, flag int, perm os.FileMode) (*File, error) // Close unlocks and closes the underlying file. From 0a585018a98f415fd19fc7a3360ac59c4ba242f5 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 14:44:21 +0200 Subject: [PATCH 07/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 43 ++++++++++++++++++++--- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index 8c5bf163..71b2bd19 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -83,6 +83,37 @@ func Open(name string) (*File, error) // is write-locked; otherwise, it is read-locked. func OpenFile(name string, flag int, perm os.FileMode) (*File, error) +// Read reads up to len(b) bytes from the File. +// It returns the number of bytes read and any error encountered. +// At end of file, Read returns 0, io.EOF. +// +// File can be read-locked or write-locked. +func (f *File) Read(b []byte) (n int, err error) + +// ReadAt reads len(b) bytes from the File starting at byte offset off. +// It returns the number of bytes read and the error, if any. +// ReadAt always returns a non-nil error when n < len(b). +// At end of file, that error is io.EOF. +// +// File can be read-locked or write-locked. +func (f *File) ReadAt(b []byte, off int64) (n int, err error) + +// Write writes len(b) bytes to the File. +// It returns the number of bytes written and an error, if any. +// Write returns a non-nil error when n != len(b). +// +// If File is not write-locked Write returns an error. +func (f *File) Write(b []byte) (n int, err error) + +// WriteAt writes len(b) bytes to the File starting at byte offset off. +// It returns the number of bytes written and an error, if any. +// WriteAt returns a non-nil error when n != len(b). +// +// If file was opened with the O_APPEND flag, WriteAt returns an error. +// +// If File is not write-locked WriteAt returns an error. +func (f *File) WriteAt(b []byte, off int64) (n int, err error) + // Close unlocks and closes the underlying file. // // Close may be called multiple times; all calls after the first will return a @@ -121,12 +152,16 @@ func (mu *Mutex) String() string ## Rationale -The golang/go/src/cmd/go/internal/lockedfile already exists but has untrusted & -unmaintained alternatives. +* The `lockedfile.File` implements a subset of the `os.File` but with file + locking protection. + +* `lockedfile` adds an `Edit` function; `Edit` is not currently part of the + `file` package. Edit exists to make it easier to implement locked + read-modify-write operation. -* Exporting this package public will make it more used. A tiny surge of issues +* Making this package public will make it more used. A tiny surge of issues might come in the beginning; at the benefits of everyone. ( Unless it's bug - free !! ). + free !!). * There exists a https://godoc.org/github.com/rogpeppe/go-internal package that exports a lot of internal packages from the go repo. But if go-internal From 5bbc00764702b2a3a7a7212d7e69b2cf42fd6789 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 14:54:00 +0200 Subject: [PATCH 08/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index 71b2bd19..a86772f3 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -132,7 +132,10 @@ func (f *File) Close() error // must not be copied after first use. The Path field must be set before first // use and must not be change thereafter. type Mutex struct { - Path string // The path to the well-known lock file. Must be non-empty. + // Path to the well-known lock file. Must be non-empty. + // + // Path must not change on a locked mutex. + Path string // contains filtered or unexported fields } @@ -155,6 +158,10 @@ func (mu *Mutex) String() string * The `lockedfile.File` implements a subset of the `os.File` but with file locking protection. +* The `lockedfile.Mutex` does not implement `sync.Locker`: unlike a + `sync.Mutex`, a `lockedfile.Mutex` can fail to lock (e.g. if there is a + permission error in the filesystem). + * `lockedfile` adds an `Edit` function; `Edit` is not currently part of the `file` package. Edit exists to make it easier to implement locked read-modify-write operation. From e410302e9e40f5f90ca2f3b18e6eab07936c31c1 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 14:59:05 +0200 Subject: [PATCH 09/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index a86772f3..032cdfc1 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -187,4 +187,7 @@ Perhaps that implies that this should go in the `x/sys` or `x/sync` repo instead ## Implementation Adrien Delorme plans to do copy the exported types in the proposal section from - `cmd/go/internal/lockedfile` to `x/sync`. \ No newline at end of file + `cmd/go/internal/lockedfile` to `x/sync`. + +Adrien Delorme plans to change the references to the `lockedfile` package in +`cmd`. \ No newline at end of file From 3a99c89857dc156913192aed411805ed14bcf1f0 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 17 Oct 2019 14:59:48 +0200 Subject: [PATCH 10/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index 032cdfc1..eab8b174 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -9,7 +9,7 @@ Discussion at https://golang.org/issue/33974. ## Abstract Move already existing code residing in -`golang/go/src/cmd/go/internal/lockedfile` to `x/exp`. At `x/exp/lockedfile`. +`golang/go/src/cmd/go/internal/lockedfile` to `x/sync`. ## Background From 9b55e8fcf6ba82db7669272bb0b8b41c133435e1 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 8 Nov 2019 17:23:51 +0100 Subject: [PATCH 11/14] Update 33974-add-public-lockedfile-pkg.md add new func --- design/33974-add-public-lockedfile-pkg.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index eab8b174..1812e607 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -75,6 +75,15 @@ func Create(name string) (*File, error) // The associated file descriptor has mode O_RDWR and the file is write-locked. func Edit(name string) (*File, error) +// Transform invokes t with the result of reading the named file, with its lock +// still held. +// +// If t returns a nil error, Transform then writes the returned contents back to +// the file, making a best effort to preserve existing contents on error. +// +// t must not modify the slice passed to it. +func Transform(name string, t func([]byte) ([]byte, error)) (err error) { + // Open is like os.Open, but returns a read-locked file. func Open(name string) (*File, error) From 402cae2bed5974cffd80606b9625dcf2fd3ed9ed Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Fri, 8 Nov 2019 17:27:44 +0100 Subject: [PATCH 12/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index 1812e607..56648ba2 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -171,9 +171,11 @@ func (mu *Mutex) String() string `sync.Mutex`, a `lockedfile.Mutex` can fail to lock (e.g. if there is a permission error in the filesystem). -* `lockedfile` adds an `Edit` function; `Edit` is not currently part of the - `file` package. Edit exists to make it easier to implement locked - read-modify-write operation. +* `lockedfile` adds an `Edit` and a `Transform` function; `Edit` is not + currently part of the `file` package. Edit exists to make it easier to + implement locked read-modify-write operation. Transform simplifies the act of + reading and then writing to a locked file. + * Making this package public will make it more used. A tiny surge of issues might come in the beginning; at the benefits of everyone. ( Unless it's bug From 6db43b109063edb7cd85573f2f8485175fbef9d5 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 20 Feb 2020 15:31:43 +0100 Subject: [PATCH 13/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 25 ++++++++++++----------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index 56648ba2..acd7ce5d 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -15,10 +15,9 @@ Move already existing code residing in A few open source Go projects are implementing file locking mechanisms but they do not seem to be maintained anymore: -* https://github.com/gofrs/flock : This repo has accepted PRs as recently as - this March, so this implementation may be maintained — but it is not (yet?) - portable to as many platforms as the implementation in the Go project, and - we could argue that our `lockedfile` package API is more ergonomic anyway. +* https://github.com/gofrs/flock : This repo has lastly accepted PRs in March + 2019, so this implementation may be maintained and we could argue that the + `lockedfile` package API is more ergonomic. * https://github.com/juju/fslock : Note that this implementation is both unmaintained and LGPL-licensed, so even folks who would like to use it might not be able to. Also not that this repo [was selected for removal in @@ -82,14 +81,14 @@ func Edit(name string) (*File, error) // the file, making a best effort to preserve existing contents on error. // // t must not modify the slice passed to it. -func Transform(name string, t func([]byte) ([]byte, error)) (err error) { +func Transform(name string, t func([]byte) ([]byte, error)) (err error) // Open is like os.Open, but returns a read-locked file. func Open(name string) (*File, error) // OpenFile is like os.OpenFile, but returns a locked file. -// If flag implies edition (ie: os.O_TRUNC, os.O_WRONLY or os.O_RDWR), the file -// is write-locked; otherwise, it is read-locked. +// If flag implies write access (ie: os.O_TRUNC, os.O_WRONLY or os.O_RDWR), the +// file is write-locked; otherwise, it is read-locked. func OpenFile(name string, flag int, perm os.FileMode) (*File, error) // Read reads up to len(b) bytes from the File. @@ -173,12 +172,12 @@ func (mu *Mutex) String() string * `lockedfile` adds an `Edit` and a `Transform` function; `Edit` is not currently part of the `file` package. Edit exists to make it easier to - implement locked read-modify-write operation. Transform simplifies the act of - reading and then writing to a locked file. + implement locked read-modify-write operation. `Transform` simplifies the act + of reading and then writing to a locked file. * Making this package public will make it more used. A tiny surge of issues - might come in the beginning; at the benefits of everyone. ( Unless it's bug + might come in the beginning; at the benefits of everyone. (Unless it's bug free !!). * There exists a https://godoc.org/github.com/rogpeppe/go-internal package that @@ -189,10 +188,12 @@ func (mu *Mutex) String() string ## Compatibility -There are no compatibility issues. Since this will be a code addition but -Ideally we don't want to maintain two copies of this package going forward, and +There are no retro-compatibility issues since this will be a code addition but +ideally we don't want to maintain two copies of this package going forward, and we probably don't want to vendor `x/exp` into the `cmd` module. + + Perhaps that implies that this should go in the `x/sys` or `x/sync` repo instead? ## Implementation From d6be79e2bc2e232ab1ed7990ec36219f8b427040 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 20 Feb 2020 15:34:24 +0100 Subject: [PATCH 14/14] Update 33974-add-public-lockedfile-pkg.md --- design/33974-add-public-lockedfile-pkg.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/design/33974-add-public-lockedfile-pkg.md b/design/33974-add-public-lockedfile-pkg.md index acd7ce5d..e54c6b3b 100644 --- a/design/33974-add-public-lockedfile-pkg.md +++ b/design/33974-add-public-lockedfile-pkg.md @@ -17,7 +17,9 @@ A few open source Go projects are implementing file locking mechanisms but they do not seem to be maintained anymore: * https://github.com/gofrs/flock : This repo has lastly accepted PRs in March 2019, so this implementation may be maintained and we could argue that the - `lockedfile` package API is more ergonomic. + `lockedfile` package API is more ergonomic. Incompatibilities with AIX, + Solaris and Illumos are preventing file locking on both projects, but it + looks like the go team is addressing for `lockedfile`. * https://github.com/juju/fslock : Note that this implementation is both unmaintained and LGPL-licensed, so even folks who would like to use it might not be able to. Also not that this repo [was selected for removal in