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

[enh] Wait for dpkg lock to be free #571

Merged
merged 3 commits into from Nov 20, 2018

Conversation

Projects
None yet
4 participants
@maniackcrudelis
Contributor

maniackcrudelis commented Nov 6, 2018

The problem

We have a lot of failed upgrade because of apt couldn't get its lock file because of another apt running in background.

Solution

Add an internal helper to check before any apt helper if the lock is free. Or wait to run apt.

PR Status

Tested on a VM
Ready to be reviewed.

How to test

Run an apt command that will hang, waiting for an answer from the user. Like an install or an upgrade.
Then try to use an apt helper.

Validation

  • Principle agreement 2/2 : frju365, ljf
  • Quick review 1/1 : Josué
  • Simple test 0/1 :
  • Deep review 1/1 : ljf
# [internal]
#
# usage: ynh_is_dpkg_free
ynh_is_dpkg_free() {

This comment has been minimized.

@Josue-T

Josue-T Nov 6, 2018

Contributor

Is it better to use a name like ynh_wait_dpkg_free to explain that it's not just a check.

This comment has been minimized.

@maniackcrudelis

maniackcrudelis Nov 6, 2018

Contributor

Honestly, I don't care about the name of the helper.

This comment has been minimized.

@zamentur

zamentur Nov 20, 2018

Contributor

Change done

for try in `seq 1 17`
do
# Check if /var/lib/dpkg/lock is used by another process
if sudo lsof /var/lib/dpkg/lock > /dev/null

This comment has been minimized.

@Josue-T

Josue-T Nov 6, 2018

Contributor

Does we really need sudo as normally all helper should be called as root ?

This comment has been minimized.

@maniackcrudelis

maniackcrudelis Nov 6, 2018

Contributor

lsof won't work if it's not root, I added it for my own tests and I forgot to remove it...
Still, It's maybe not useless if an user want to use it on his own.
The problem is, without root, lsof will just don't see any process using the lock. So I'd rather prefer to keep it, even if it's going to be useless in most cases.

This comment has been minimized.

@zamentur

zamentur Nov 20, 2018

Contributor

Note lsof command is here because mariadb is installed. Might we add this requirement in our packages in case of mariadb will be remove in future ?

This comment has been minimized.

@maniackcrudelis

maniackcrudelis Nov 20, 2018

Contributor

I have this package on my own debian (buster though), but I guess it could be better to have it as a dependency.

@Josue-T

Josue-T approved these changes Nov 6, 2018

@frju365

This comment has been minimized.

Member

frju365 commented Nov 15, 2018

LGTM for me (principle argreement) but as I can't edit post, I can't tick the box. ;)

@Josue-T

This comment has been minimized.

Contributor

Josue-T commented Nov 15, 2018

LGTM for me (principle argreement) but as I can't edit post, I can't tick the box. ;)

I wrote your name in the "principle argreement".

@frju365

This comment has been minimized.

Member

frju365 commented Nov 15, 2018

Thanks ;)

@maniackcrudelis maniackcrudelis merged commit b3c5f44 into stretch-unstable Nov 20, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@maniackcrudelis maniackcrudelis deleted the wait_for_dpkg_lock branch Nov 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment