Go: proposal: make the internal lockedfile package public

[azr]

An internal filelock pkg can be found here: https://godoc.org/github.com/golang/go/src/cmd/go/internal/lockedfile/internal/filelock

A few projects are implementing file locking [1] but they do not seem to be maintained and I think they are not as nice as the internal filelock pkg I mentioned before. As a result some projects are doing their own version of it [2].

I suggest we make the filelock pkg public as I think this could be beneficial to the mass.

I'd be glad to do it given some pointers, like where to put it ?

Thanks !

---

[1] File lock projects:

https://github.com/gofrs/flock
https://github.com/MichaelS11/go-file-lock
https://github.com/juju/fslock

[2] Projects that implement their own file locking:

terraform

boltdb

[mvdan]

Why not use the exported version at https://godoc.org/github.com/rogpeppe/go-internal? :)

[azr]

Hey @mvdan, 🙂 here are a few reasons why I didn't go with it:

• the filelock of go-internal is in an internal pkg
• importing go-internal makes my go-deps bigger than necessary ( edit: this not such a big issue )
• it feels wrong, if I find a bug, then I have to make a pull request to go and then wait for it to get merged and then wait for go-internals to update. It sounds like it has all the annoying effects of having these pkgs not internal anyway.

[mvdan]

the filelock of go-internal is in an internal pkg

I'm not sure why that is, perhaps @rogpeppe or @myitcv can answer. Have you looked at https://godoc.org/github.com/rogpeppe/go-internal/lockedfile? That seems like a public, higher-level version of it.

That aside, sure, there are some upsides to having this in the standard library. But I think the downsides are generally greater:

• Once a package is public, it can never have backwards incompatible changes
• The standard library can grow, but never shrink in size
• Once the number of users greatly outgrows just cmd/go, the maintainers will probably have extra work responding to issues, reviewing patches, and fixing bugs that might not even affect cmd/go

I think a far better proposal would be to include this in one of the official external repos, like x/sync. Once that's worked well for a while, then it can be part of the public standard library. This is the path that context took, for example.

[azr]

lockedfile fits right to our usage and lockedfile doesn't have the filelock.ErrNotSupported. But yes lockedfile _could_ work here.

Yes, I agree this should probably be in an x/ pkg 🙂, I only suggested we make the filelock pkg public and agree this would be a bit quick to put in the stdlib.

[ianlancetaylor]

CC @bcmills

[bcmills]

At this point we've been using lockedfile for a full release cycle and it seems mostly fine, modulo what appears to be an AIX kernel bug (#32817). I'd be fine with making it public.

Then the question is where to put it: x/sys, x/exp, x/sync, or someplace else entirely?

[azr]

It's right at the intersection of sync and sys.
I _think_ lockedfile should be in x/sync/lockedfile
I _think_ that filelock is a bit lower level and could fit in x/sys/filelock ( if we ever make it public ). But these two are may be a bit redundant.

edit: regroup them both under x/exp/syssync ?

[bcmills]

@azr, filelock should remain internal — it's much too subtle to use on its own. (In particular, if you want your program to remain portable, you have to be very careful to stick to a narrow subset of the possible modes of use.)

If folks discover other interesting use-cases for filelock, then we can probably add more ergonomic packages to address those use-cases while still keeping filelock internal.

[rsc]

Given that the author of filelock says it is not ready to be exported, it seems like this is a likely decline.

Leaving open for a week for final comments.

[bcmills]

To be clear: I support making the lockedfile package itself public — I just want to ensure that the lockedfile/internal/filelock package remains internal to it.

[azr]

I just want to ensure that the lockedfile/internal/filelock package remains internal to it.

That's fine by me 🙂.

I think I'll put it in x/sync/lockedfile soonish; (unless you think another place is better ).

[rsc]

I misunderstood. Thanks for the clarification, @bcmills. Removing the FinalCommentPeriod label.

[rsc]

I don't think we are ready to add lockedfile to the standard library. It is a peculiar set of functions that does not really line up with the usual things in either os or io. Maybe it would be OK in x/sync but maybe it would be better to start in x/exp to understand if the API needs any adjustments.

If we are going to add a new public package (even in x/exp), I think the next step would be for someone to write a proper proposal doc laying out the API and explaining the decisions, get comments, and so on.

@azr, do you want to do that?

[azr]

@rsc gotcha ! Yes I want to ! Where should such a proposal doc be written ? Is there a template for that ?

[bcmills]

@azr, see https://github.com/golang/proposal#design-documents

[rsc]

Putting proposal on hold for design doc.

[azr]

Hello there; I created the proposal ( first ever ! ) here https://github.com/golang/proposal/pull/21; I hope it's good enough please feel free to give feedback 🙂 🙂 !

[gopherbot]

Change https://golang.org/cl/201277 mentions this issue: design: add proposal doc for 33974

[gopherbot]

Change https://golang.org/cl/222277 mentions this issue: cmd/go/internal/modload: use a global lockfile to avoid spurious EDEADLK on AIX and Solaris

[bcmills]

Design doc added at https://golang.org/design/33974-add-public-lockedfile-pkg.

[bcmills]

We also now have a solution for the AIX and Solaris EDEADLK bug pending (CL 222277). The workaround is simple (a retry loop), but the justification and regression test are at least non-trivial.

[azr]

Hello @bcmills, can I already start working on this ? Can I help in any manner ? Thanks !