Go: syscall: remove AIX implementation of Flock

Created on 3 Dec 2018  Â·  15Comments  Â·  Source: golang/go

https://golang.org/cl/138720 added an AIX implementation of syscall.Flock using the fcntl system call.

We rejected a similar implementation for Solaris in #24684 on the grounds that fcntl and flock are semantically different APIs (see https://github.com/golang/go/issues/24684#issuecomment-379887598).

(Note that per https://github.com/golang/go/issues/21410#issuecomment-322219677, some versions of Illumos — a Solaris variant — also support a proper flock implementation: https://www.illumos.org/issues/3252.)

We should remove the AIX Flock implementation before the 1.12 release. If we decide to add it later, we should ensure a consistent approach between Solaris and AIX.

(CC @Helflym @ianlancetaylor @rsc @tklauser)

FrozenDueToAge NeedsFix release-blocker

Most helpful comment

Yes, I do agree, AIX isn't perfect on BSD syscalls... Flock isn't needed anywhere inside the stdlib expect for cmd/go/internal/lockedfile/internal/filelock where AIX can use the Solaris version with FcntlFlock. So I think it can be removed without any problems.

All 15 comments

...or did Flock already get released in 1.11?

AIX has a flock syscall (cf https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.aix.basetrf1/lockfx.htm) and it also calls fcntl almost as it is done in the syscall package.

However, I do understand that using a fake Flock can be confusing.
@bcmills it's already released in the gcccgo frontend. But a lot of thing has changed since the port of aix/ppc64 on the gc version. So I don't think it really matters.

That page says:

The flock subroutine locks and unlocks entire files. This is a limited interface maintained for BSD compatibility, although its behavior differs from BSD in a few subtle ways.

The existence of a C compatibility shim is not, to my mind, a good reason to have the same compatibility shim in Go, given that portable Go programs already need to distinguish between a proper BSD flock and a POSIX fcntl (at least if they want to build on solaris).

And Go 1 compatibility makes it much easier to add functions in the future than to remove them: I think we should remove it for now, and we can always consider adding it back in the future if there is a compelling need.

(This sort of shim might be better suited to golang.org/x/sys/unix, which IIRC does not have the same compatibility constraints as the standard-library syscall package.)

Yes, I do agree, AIX isn't perfect on BSD syscalls... Flock isn't needed anywhere inside the stdlib expect for cmd/go/internal/lockedfile/internal/filelock where AIX can use the Solaris version with FcntlFlock. So I think it can be removed without any problems.

(This sort of shim might be better suited to golang.org/x/sys/unix, which IIRC does not have the same compatibility constraints as the standard-library syscall package.)

Removing Flock for aix from syscall SGTM. We can still add it to x/sys/unix in case something outside the Go standard library needs it.

@Helflym is planning to send a CL to move filelock over to fcntl (https://github.com/golang/go/issues/29065#issuecomment-443772917). One or the other of us can remove Flock once that lands (or even in the same CL).

Great, thanks. Let's wait for that CL to land.

Change https://golang.org/cl/152397 mentions this issue: syscall, cmd/go/internal/lockedfile: remove Flock syscall for aix/ppc64

it's already released in the gcccgo frontend.

There seems to be no GCC release with this gccgo change yet. Should we remove syscall.Flock for aix from gccgo as well so it is consistent with gc?

/cc @ianlancetaylor

There seems to be no GCC release with this gccgo change yet.

Sorry, please disregard - I checked in an outdated GCC copy. GCC 8.1 and GCC 8.2 already contain the change, so I guess we cannot remove it.

By the way, since Solaris, AIX or Windows can't use Flock, a user will have to create a lockfile_unix.go, lockfile_fcntl.go and so on if he wants to create a package with a filelock feature as it's done inside cmd/go/internal/lockedfile.
Should not Go provide such functions inside os package ? I would be better than to have all these files which might not work under specific OS.

Should not Go provide such functions inside os package?

I think filelock is too subtle for the standard library. However, I will probably propose moving lockedfile to the standard library for 1.13 or 1.14.

(Probably best to give it a release or two to work out the rough edges before we expose it to the world — and to Go 1 compatibility.)

It's OK to remove from gccgo. In practice programs expect the gc toolchain's syscall package, so it's OK to remove names from gccgo's syscall package if they aren't in gc's package.

Change https://golang.org/cl/152557 mentions this issue: syscall: remove Flock for aix/ppc64

Was this page helpful?
0 / 5 - 0 ratings