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

Add lock/unlock for build git, build, and install #479

Merged
merged 17 commits into from
Oct 19, 2023
Merged

Add lock/unlock for build git, build, and install #479

merged 17 commits into from
Oct 19, 2023

Conversation

paulo-ferraz-oliveira
Copy link
Contributor

@paulo-ferraz-oliveira paulo-ferraz-oliveira commented Oct 16, 2023

Description

Closes #187.

I've also reviewed all the scattered exit statements to turn them into return (and act on that) in the scope of the changes. A few non- top-level exit 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:

  1. inside the function where the lock shall occur, create a global variable with the folder name where the lock will reside, used by the "(un)lock" functions
  2. two "types" of locks are available: build and install
  3. when execution starts, close lock
  4. when execution ends, normally or with error (but not interrupted), open lock
  5. when execution is interrupted lock remains and will be signalled, via an error, on the next run for conditions that produce a similar lock
  6. acted on return instead of exit to ease debugging code flow and maintenance

@paulo-ferraz-oliveira paulo-ferraz-oliveira marked this pull request as ready for review October 16, 2023 02:13
kerl Show resolved Hide resolved
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!"
Copy link
Collaborator

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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"
Copy link
Collaborator

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

kerl Show resolved Hide resolved
kerl Show resolved Hide resolved
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
Copy link
Collaborator

@jadeallenx jadeallenx left a comment

Choose a reason for hiding this comment

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

lgtm

@paulo-ferraz-oliveira paulo-ferraz-oliveira changed the title Add lock/unlock for "build git" Add lock/unlock for build git, build, and install Oct 19, 2023
@paulo-ferraz-oliveira paulo-ferraz-oliveira merged commit d63e4f0 into kerl:master Oct 19, 2023
9 checks passed
@paulo-ferraz-oliveira paulo-ferraz-oliveira deleted the feature/execution-locks branch October 19, 2023 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kerl should make it easy for automation to figure out when its completed a build
2 participants