From c0a4551a2f0f2311cc91aa88a1dc2d888ac2cb8f Mon Sep 17 00:00:00 2001 From: Ayan George Date: Fri, 22 Sep 2017 15:33:55 -0400 Subject: [PATCH 1/6] Disable file locking when DEPNOLOCK set * Add DisableLocking bool members to Ctx and gps.SourceManagerConfig structs. This effectively communicates DEPNOLOCK from the shell, to Ctx, to SourceManager. The member is named DisableLocking to make its zero-value useful. * Add locker interface which implements TryLock(), Unlock(), and GetOwner() which lockfile.Lockfile alredy adheres to. This interface replaces the new type for the lf member of the SourceMgr struct. * Add a FalseLocker type which adheres to the Locker interface which does nothing. * Conditionally set the lf member of SourceMgr to either an instance of lockfile.Lockfile or FalseLocker depending on the value of SourceManagerConfig.DisableLocking. Signed-off-by: Ayan George --- cmd/dep/main.go | 7 +++-- context.go | 16 +++++----- internal/gps/source_manager.go | 55 ++++++++++++++++++++++++++++++---- 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/cmd/dep/main.go b/cmd/dep/main.go index bb66c8e639..f465197028 100644 --- a/cmd/dep/main.go +++ b/cmd/dep/main.go @@ -147,9 +147,10 @@ func (c *Config) Run() (exitCode int) { // Set up dep context. ctx := &dep.Ctx{ - Out: outLogger, - Err: errLogger, - Verbose: *verbose, + Out: outLogger, + Err: errLogger, + Verbose: *verbose, + DisableLocking: getEnv(c.Env, "DEPNOLOCK") != "", } GOPATHS := filepath.SplitList(getEnv(c.Env, "GOPATH")) diff --git a/context.go b/context.go index f0a80473ab..82454840b5 100644 --- a/context.go +++ b/context.go @@ -34,11 +34,12 @@ import ( // } // type Ctx struct { - WorkingDir string // Where to execute. - GOPATH string // Selected Go path, containing WorkingDir. - GOPATHs []string // Other Go paths. - Out, Err *log.Logger // Required loggers. - Verbose bool // Enables more verbose logging. + WorkingDir string // Where to execute. + GOPATH string // Selected Go path, containing WorkingDir. + GOPATHs []string // Other Go paths. + Out, Err *log.Logger // Required loggers. + Verbose bool // Enables more verbose logging. + DisableLocking bool // When set, disables locking. } // SetPaths sets the WorkingDir and GOPATHs fields. If GOPATHs is empty, then @@ -87,8 +88,9 @@ func defaultGOPATH() string { // initialized to log to the receiver's logger. func (c *Ctx) SourceManager() (*gps.SourceMgr, error) { return gps.NewSourceManager(gps.SourceManagerConfig{ - Cachedir: filepath.Join(c.GOPATH, "pkg", "dep"), - Logger: c.Out, + Cachedir: filepath.Join(c.GOPATH, "pkg", "dep"), + Logger: c.Out, + DisableLocking: c.DisableLocking, }) } diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index 38b3dd8281..cd2955b5d5 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -28,6 +28,41 @@ import ( // Used to compute a friendly filepath from a URL-shaped input. var sanitizer = strings.NewReplacer("-", "--", ":", "-", "/", "-", "+", "-") +// A locker is responsible for preventing multiple instances of dep from interfereing +// with one-another. +// +// Currently, anything that can either TryLock(), Unlock(), or GetOwner() satifies +// that need. +type locker interface { + TryLock() error + Unlock() error + GetOwner() (*os.Process, error) +} + +// A falselocker adheres to the locker inteface and it's purpose is to quietly +// fail to lock when the DEPNOLOCK environment variable is set. +// +// This allows dep to run on systems where file locking doesn't work -- +// particularly those that use union mount type filesystems that don't +// implement hard links or fnctl() style locking. +type falseLocker struct{} + +// Always returns an error to indicate there's no current ower PID for our +// lock. +func (fl falseLocker) GetOwner() (*os.Process, error) { + return nil, fmt.Errorf("falseLocker always fails") +} + +// Does nothing and returns a nil error so caller beleives locking succeeded. +func (fl falseLocker) TryLock() error { + return nil +} + +// Does nothing and returns a nil error so caller beleives unlocking succeeded. +func (fl falseLocker) Unlock() error { + return nil +} + // A SourceManager is responsible for retrieving, managing, and interrogating // source repositories. Its primary purpose is to serve the needs of a Solver, // but it is handy for other purposes, as well. @@ -121,7 +156,7 @@ func (p ProjectAnalyzerInfo) String() string { // tools; control via dependency injection is intended to be sufficient. type SourceMgr struct { cachedir string // path to root of cache dir - lf *lockfile.Lockfile // handle for the sm lock file on disk + lf locker // handle for the sm lock file on disk suprvsr *supervisor // subsystem that supervises running calls/io cancelAll context.CancelFunc // cancel func to kill all running work deduceCoord *deductionCoordinator // subsystem that manages import path deduction @@ -142,8 +177,9 @@ var _ SourceManager = &SourceMgr{} // SourceManagerConfig holds configuration information for creating SourceMgrs. type SourceManagerConfig struct { - Cachedir string // Where to store local instances of upstream sources. - Logger *log.Logger // Optional info/warn logger. Discards if nil. + Cachedir string // Where to store local instances of upstream sources. + Logger *log.Logger // Optional info/warn logger. Discards if nil. + DisableLocking bool // True if dep SourceManager shoud NOT lock. } // NewSourceManager produces an instance of gps's built-in SourceManager. @@ -175,7 +211,14 @@ func NewSourceManager(c SourceManagerConfig) (*SourceMgr, error) { // we can spin on. glpath := filepath.Join(c.Cachedir, "sm.lock") - lockfile, err := lockfile.New(glpath) + + lockfile, err := func() (locker, error) { + if c.DisableLocking { + return falseLocker{}, nil + } + return lockfile.New(glpath) + }() + if err != nil { return nil, CouldNotCreateLockError{ Path: glpath, @@ -214,7 +257,7 @@ func NewSourceManager(c SourceManagerConfig) (*SourceMgr, error) { // The first time this is evaluated, duration will be very large as lasttime is 0. // Unless time travel is invented and someone travels back to the year 1, we should // be ok. - if duration > 15*time.Second { + if duration > 14*time.Second { fmt.Fprintf(os.Stderr, "waiting for lockfile %s: %s\n", glpath, err.Error()) lasttime = nowtime } @@ -238,7 +281,7 @@ func NewSourceManager(c SourceManagerConfig) (*SourceMgr, error) { sm := &SourceMgr{ cachedir: c.Cachedir, - lf: &lockfile, + lf: lockfile, suprvsr: superv, cancelAll: cf, deduceCoord: deducer, From 00881196d1364bb8eaa17fe3c88349164a1321fc Mon Sep 17 00:00:00 2001 From: Ayan George Date: Wed, 27 Sep 2017 16:23:58 -0400 Subject: [PATCH 2/6] Revert stray edit. Signed-off-by: Ayan George --- internal/gps/source_manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index cd2955b5d5..5a8ae3787f 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -257,7 +257,7 @@ func NewSourceManager(c SourceManagerConfig) (*SourceMgr, error) { // The first time this is evaluated, duration will be very large as lasttime is 0. // Unless time travel is invented and someone travels back to the year 1, we should // be ok. - if duration > 14*time.Second { + if duration > 15*time.Second { fmt.Fprintf(os.Stderr, "waiting for lockfile %s: %s\n", glpath, err.Error()) lasttime = nowtime } From 3faed1071b3219e6706bed1d884315c2e894fba1 Mon Sep 17 00:00:00 2001 From: Ayan George Date: Thu, 28 Sep 2017 09:24:43 -0400 Subject: [PATCH 3/6] Improve comment for DisableLocking Signed-off-by: Ayan George --- context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context.go b/context.go index 82454840b5..ba8007360e 100644 --- a/context.go +++ b/context.go @@ -39,7 +39,7 @@ type Ctx struct { GOPATHs []string // Other Go paths. Out, Err *log.Logger // Required loggers. Verbose bool // Enables more verbose logging. - DisableLocking bool // When set, disables locking. + DisableLocking bool // When set, no lock file will be created to protect against simultaneous dep processes. } // SetPaths sets the WorkingDir and GOPATHs fields. If GOPATHs is empty, then From 744d239b7b2774713385e263e64447883c625144 Mon Sep 17 00:00:00 2001 From: Ayan George Date: Fri, 29 Sep 2017 09:46:19 -0400 Subject: [PATCH 4/6] Fix comment type-os --- internal/gps/source_manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index 5a8ae3787f..8f6f8a0b58 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -39,7 +39,7 @@ type locker interface { GetOwner() (*os.Process, error) } -// A falselocker adheres to the locker inteface and it's purpose is to quietly +// A falselocker adheres to the locker interface and its purpose is to quietly // fail to lock when the DEPNOLOCK environment variable is set. // // This allows dep to run on systems where file locking doesn't work -- From 0dec1c3b02dc25378af01bb2acddae83258012b5 Mon Sep 17 00:00:00 2001 From: Ayan George Date: Fri, 29 Sep 2017 09:46:19 -0400 Subject: [PATCH 5/6] Fix comment type-os Signed-off-by: Ayan George --- internal/gps/source_manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index 5a8ae3787f..8f6f8a0b58 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -39,7 +39,7 @@ type locker interface { GetOwner() (*os.Process, error) } -// A falselocker adheres to the locker inteface and it's purpose is to quietly +// A falselocker adheres to the locker interface and its purpose is to quietly // fail to lock when the DEPNOLOCK environment variable is set. // // This allows dep to run on systems where file locking doesn't work -- From 0b746f97d8241c88d313135ea807bc2c40d1a3e9 Mon Sep 17 00:00:00 2001 From: Ayan George Date: Fri, 29 Sep 2017 16:49:29 -0400 Subject: [PATCH 6/6] Fix yet more type-os. Signed-off-by: Ayan George --- internal/gps/source_manager.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index 8f6f8a0b58..23f3c1c8b5 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -28,11 +28,11 @@ import ( // Used to compute a friendly filepath from a URL-shaped input. var sanitizer = strings.NewReplacer("-", "--", ":", "-", "/", "-", "+", "-") -// A locker is responsible for preventing multiple instances of dep from interfereing -// with one-another. +// A locker is responsible for preventing multiple instances of dep from +// interfering with one-another. // -// Currently, anything that can either TryLock(), Unlock(), or GetOwner() satifies -// that need. +// Currently, anything that can either TryLock(), Unlock(), or GetOwner() +// satifies that need. type locker interface { TryLock() error Unlock() error @@ -179,7 +179,7 @@ var _ SourceManager = &SourceMgr{} type SourceManagerConfig struct { Cachedir string // Where to store local instances of upstream sources. Logger *log.Logger // Optional info/warn logger. Discards if nil. - DisableLocking bool // True if dep SourceManager shoud NOT lock. + DisableLocking bool // True if the SourceManager should NOT use a lock file to protect the Cachedir from multiple processes. } // NewSourceManager produces an instance of gps's built-in SourceManager.