code review 6446087: leveldb: file locking for OSX/amd64.

leveldb: file locking for OSX/amd64.

This was tested by running two instances of the program in manualtest/filelock.

Also add a very basic test to ease implementation for other systems.

[nictuku, 2012-08-04 20:10:06]

Hello nigeltao@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/leveldb-go

[nictuku, 2012-08-04 22:26:29]

http://codereview.appspot.com/6446087/diff/5001/leveldb/db/file_lock_generic.go
File leveldb/db/file_lock_generic.go (right):

http://codereview.appspot.com/6446087/diff/5001/leveldb/db/file_lock_generic....
leveldb/db/file_lock_generic.go:5: // +build !linux,!amd64 !darwin,!amd64
It looks like this is not really what I need.

My next patch will add arm support, and this pattern ends up matching linux,arm
too, unintentionally.

I can't think of a solution other than dropping this file entirely.

[nigeltao, 2012-08-06 00:21:42]

http://codereview.appspot.com/6446087/diff/3005/leveldb/db/file_test.go
File leveldb/db/file_test.go (right):

http://codereview.appspot.com/6446087/diff/3005/leveldb/db/file_test.go#newcode1
leveldb/db/file_test.go:1: package db
Missing copyright notice.

But I wouldn't bother with this file. There's already
manualtest/filelock/main.go, which actually tests for exclusivity.

[nigeltao, 2012-08-06 00:40:45]

On 5 August 2012 08:26,  <yves.junqueira@gmail.com> wrote:
>
>
http://codereview.appspot.com/6446087/diff/5001/leveldb/db/file_lock_generic.go
> File leveldb/db/file_lock_generic.go (right):
>
>
http://codereview.appspot.com/6446087/diff/5001/leveldb/db/file_lock_generic....
> leveldb/db/file_lock_generic.go:5: // +build !linux,!amd64
> !darwin,!amd64
> It looks like this is not really what I need.
>
> My next patch will add arm support, and this pattern ends up matching
> linux,arm too, unintentionally.

I think you want
// +build !linux,!darwin !amd64
now and
// Match everything except: darwin_amd64, darwin_arm, linux_amd64.
// +build !linux,!darwin !amd64,!arm linux,arm
later.

See http://golang.org/pkg/go/build/#Build_Constraints but there is
currently a typo on that page (http://codereview.appspot.com/6306076).
If you have golang tip checked out, you can run "godoc go/build"
instead.


Also, the doc comment on the Lock method, in levedb/db/file.go, needs updating.

[nictuku, 2012-08-06 12:19:25]

Please take another look.

http://codereview.appspot.com/6446087/diff/3005/leveldb/db/file_test.go
File leveldb/db/file_test.go (right):

http://codereview.appspot.com/6446087/diff/3005/leveldb/db/file_test.go#newcode1
leveldb/db/file_test.go:1: package db
On 2012/08/06 00:21:42, nigeltao wrote:
> Missing copyright notice.
> 
> But I wouldn't bother with this file. There's already
> manualtest/filelock/main.go, which actually tests for exclusivity.

Dropped the file.

[nigeltao, 2012-08-07 01:02:40]

LGTM.

[nigeltao, 2012-08-07 01:05:57]

*** Submitted as
http://code.google.com/p/leveldb-go/source/detail?r=f9c56ee746b1 ***

leveldb: file locking for OSX/amd64.

This was tested by running two instances of the program in manualtest/filelock.

Also add a very basic test to ease implementation for other systems.

R=nigeltao
CC=golang-dev
http://codereview.appspot.com/6446087

Committer: Nigel Tao <nigeltao@golang.org>