Wait for db.lck to become available before starting a db operation #573
Conversation
exec.go
Outdated
| // 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) { |
There was a problem hiding this comment.
This should be join pacmanconf.DbPath + db.lck
| case "R", "remove": | ||
| return true | ||
| case "S", "sync": | ||
| if parser.existsArg("y", "refresh") { |
There was a problem hiding this comment.
These are redundant, they'll be caught by the default return true at the end.
There was a problem hiding this comment.
The default is return false though, I did notice needRoot does have a lot of redundancy
There was a problem hiding this comment.
The default inside of sync is true though.
There was a problem hiding this comment.
Maybe needRoot needs a touch up as well
There was a problem hiding this comment.
Probably, been a long time since I've even looked at it.
exec.go
Outdated
| if _, err := os.Stat("/var/lib/pacman/db.lck"); os.IsNotExist(err) { | ||
| return nil | ||
| } | ||
| fmt.Println(bold(yellow(smallArrow)), "db.lck is present. Waiting 3 seconds and trying again") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I'll do an initial check, print the message and then loop
exec.go
Outdated
| // 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) { |
There was a problem hiding this comment.
It's possible to get different errors. After checking os.IsNotExist(err) You should have a else if for the generic err != nil.
There was a problem hiding this comment.
That else if would return nil as well and abort the lock so any error actually will return nil
There was a problem hiding this comment.
Yeah I just noticed you're not even handling errors. In that case why have a return value at all?
exec.go
Outdated
|
|
||
| for { | ||
| time.Sleep(3 * time.Second) | ||
| if _, err := os.Stat(alpmConf.DBPath + "db.lck"); err != nil { |
|
I think it's good now apart from my last comment (and that I have not actually ran the code) |
|
Testing this out I have the most nit picky ocd complaint. The cursor is put on the next line while it waits. This annoys me for some reason. Apart from that works well. What do you think about this instead? diff --git a/exec.go b/exec.go
index 2a97ae3..a5b95dc 100644
--- a/exec.go
+++ b/exec.go
@@ -60,11 +60,12 @@ func waitLock() {
return
}
- fmt.Println(bold(yellow(smallArrow)), "db.lck is present. Waiting... ")
+ fmt.Print(bold(yellow(smallArrow)), " db.lck is present. Waiting...")
for {
time.Sleep(3 * time.Second)
if _, err := os.Stat(filepath.Join(alpmConf.DBPath, "db.lck")); err != nil {
+ fmt.Println()
return
}
} |
|
That is the most nit picky ocd complaint 😆 , merge it with #576 |
On split updates some funkiness can happen where an
yayinstance is unlocked before the other one gets to the 2nd pacman call but it should be working resulting in:It should be stable enough for forced deployment without a flag though