This repository has been archived by the owner on Sep 9, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1k
[WIP] Feature to disable file locking when DEPNOLOCK set #1206
Merged
Merged
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
c0a4551
Disable file locking when DEPNOLOCK set
ayang64 0088119
Revert stray edit.
ayang64 3faed10
Improve comment for DisableLocking
ayang64 744d239
Fix comment type-os
ayang64 0dec1c3
Fix comment type-os
ayang64 f6a117f
Merge branch 'dep-no-lock' of github.com:ayang64/dep into dep-no-lock
ayang64 0b746f9
Fix yet more type-os.
ayang64 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: s/interfereing/interfering/ |
||
// 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "shoud" typo here. also, let's rewrite this a bit, too: "True if the SourceManager should NOT use a file lock to protect the Cachedir from multiple processes." |
||
} | ||
|
||
// 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, | ||
|
@@ -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, | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need a little more on this comment here. it's in reference to a rather obscure feature that most people don't know about. with just this context, i would probably expect people to assume that it referred to, say, not writing out
Gopkg.lock
(which is not something we do at all). instead, maybe:"When set, no lock file will be created to protect against multiple simultaneous dep processes."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely. Frankly I wrote something similar originally and changed my mind (something about that comment being much longer than the others -- silly I know).
I'll fix immediately.