Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait for db.lck to become available before starting a db operation #573

Merged
merged 5 commits into from
Jul 21, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ func updateSudo() {
}
}

// waitLock will lock yay checking the status of db.lck until it does not exist
func waitLock() (err error) {
for {
if _, err := os.Stat("/var/lib/pacman/db.lck"); os.IsNotExist(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be join pacmanconf.DbPath + db.lck

Copy link
Contributor

Choose a reason for hiding this comment

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

It's possible to get different errors. After checking os.IsNotExist(err) You should have a else if for the generic err != nil.

Copy link
Owner Author

Choose a reason for hiding this comment

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

That else if would return nil as well and abort the lock so any error actually will return nil

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I just noticed you're not even handling errors. In that case why have a return value at all?

Copy link
Owner Author

Choose a reason for hiding this comment

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

out of habit

return nil
}
fmt.Println(bold(yellow(smallArrow)), "db.lck is present. Waiting 3 seconds and trying again")
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I'd like it to drop the 3 second message and just say Waiting.... The timer can still be on 3 seconds and then resume when it see's the lock is gone. Avoids the spam of seeing this every 3 seconds.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'll do an initial check, print the message and then loop

time.Sleep(3 * time.Second)
}
}

func passToPacman(args *arguments) *exec.Cmd {
argArr := make([]string, 0)

Expand All @@ -71,6 +82,9 @@ func passToPacman(args *arguments) *exec.Cmd {

argArr = append(argArr, args.targets...)

if args.needWait() {
waitLock()
}
return exec.Command(argArr[0], argArr[1:]...)
}

Expand Down
44 changes: 44 additions & 0 deletions parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,50 @@ func (parser *arguments) needRoot() bool {
}
}

//needWait checks if waitLock() should be called before calling pacman
func (parser *arguments) needWait() bool {
if parser.existsArg("h", "help") {
return false
}

if parser.existsArg("p", "print") {
return false
}

switch parser.op {
case "D", "database":
return true
case "F", "files":
if parser.existsArg("y", "refresh") {
return true
}
return false
case "R", "remove":
return true
case "S", "sync":
if parser.existsArg("y", "refresh") {
Copy link
Contributor

Choose a reason for hiding this comment

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

These are redundant, they'll be caught by the default return true at the end.

Copy link
Owner Author

Choose a reason for hiding this comment

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

The default is return false though, I did notice needRoot does have a lot of redundancy

Copy link
Contributor

Choose a reason for hiding this comment

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

The default inside of sync is true though.

Copy link
Owner Author

Choose a reason for hiding this comment

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

True. 😇

Copy link
Owner Author

Choose a reason for hiding this comment

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

Maybe needRoot needs a touch up as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably, been a long time since I've even looked at it.

return true
}
if parser.existsArg("u", "sysupgrade") {
return true
}
if parser.existsArg("s", "search") {
return false
}
if parser.existsArg("l", "list") {
return false
}
if parser.existsArg("i", "info") {
return false
}
return true
case "U", "upgrade":
return true
default:
return false
}
}

func (parser *arguments) addOP(op string) (err error) {
if parser.op != "" {
err = fmt.Errorf("only one operation may be used at a time")
Expand Down