-
Notifications
You must be signed in to change notification settings - Fork 234
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
Add lock/unlock for build git, build, and install #479
Add lock/unlock for build git, build, and install #479
Conversation
This is kinda scope creep, but I found it during tests to the current branch...
The `cd` mechanism is kinda messy, but I didn't want to touch it as a whole (call it scope creep)
This'd only be visible on failure, but still it's nice to have it work as expected!
... with our expectations
kerl
Outdated
|
||
if [ "$1" = "close" ]; then | ||
if [ -f "$3/$2.lock" ]; then | ||
error "trying to $2 in $3, but lock file ($3/$2.lock) exists!" |
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.
i think if the lock file is "old" (like more than 14 days) we should remove it/ignore it
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.
Oh, yeah, I can implement that.
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.
kerl
Outdated
|
||
lock_close_build() { | ||
# $_KERL_BUILD_DIR is global | ||
lock "close" "build" "$_KERL_BUILD_DIR" |
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.
i dont understand open and close - i would prefer "lock" "unlock" lock_build
unlock_build
etc
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.
No prob. I thought of lock as a noun, not a verb, but a verb works too 👍 (so, in my mind, unlock
would be open lock
😄). I'll change it.
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.
I wanted to have lock/unlock in the same function, to "control" the filename better, but they don't seem to be so far apart that it'll make a difference
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.
lgtm
Description
Closes #187.
I've also reviewed all the scattered
exit
statements to turn them intoreturn
(and act on that) in the scope of the changes. A few non- top-levelexit
statements still remain, though.Leaving this as a draft, for the time being, since I wanna self-review it (further) still...The change
What I did, mostly, was:
return
instead ofexit
to ease debugging code flow and maintenance