Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

source_manager.go handle locking better #853

Merged
merged 6 commits into from
Jul 21, 2017
Merged

Conversation

matjam
Copy link
Contributor

@matjam matjam commented Jul 19, 2017

What does this do / why do we need it?

Adds https://github.com/nightlyone/lockfile to manage the lockfile that dep uses. Concurrent use of dep no longer fails, subsequence invocations of dep will cause the subsequence instances to wait while the first runs, and then they will continue.

Also, stale lockfiles are handled correctly.

What should your reviewer look out for in this PR?

Correctness.

Do you need help or clarification on anything?

How to use git ... ;-)

Which issue(s) does this PR fix?

fixes #820
fixes #852

Nathan Ollerenshaw added 2 commits July 18, 2017 23:55
Fix nit around capital letter in error message.
…ss. We should not allow NewSourceManager to be called twice at the same time.
@matjam matjam closed this Jul 19, 2017
@matjam matjam reopened this Jul 19, 2017
@matjam matjam closed this Jul 19, 2017
@matjam matjam reopened this Jul 19, 2017
@matjam matjam closed this Jul 19, 2017
@matjam
Copy link
Contributor Author

matjam commented Jul 19, 2017

once more, with feeling.

@matjam matjam reopened this Jul 19, 2017
Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just nits, really - it's awesome how neatly that lib fits in!

}
}

// there is a lockfile, but it's owned by someone else. We'll try to lock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: capitalize 😄

// permanently.

err = lockfile.TryLock()
for err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this loop be effectively unbounded AND not providing the user any feedback seems concerning. But, I'd be OK with addressing that in a follow-up - though, please make a comment on #534, as I think that'll be the appropriate way to tell the user this is happening.

// Fix for #820
//
// Consult https://godoc.org/github.com/nightlyone/lockfile for the lockfile
// behaviour. It's magic. It deals with stale processes, and if there is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

giphy

@matjam
Copy link
Contributor Author

matjam commented Jul 20, 2017

And close...

@matjam matjam closed this Jul 20, 2017
@matjam
Copy link
Contributor Author

matjam commented Jul 20, 2017

And open!

@matjam matjam reopened this Jul 20, 2017
@sdboyer
Copy link
Member

sdboyer commented Jul 21, 2017

ok, i think we're looking good for this. i'll open a follow-up related to refactoring the interface.

@sdboyer
Copy link
Member

sdboyer commented Jul 21, 2017

oh, and, woohoo! 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ugly user presentation when sm.lock exists due to abort
3 participants