-
Notifications
You must be signed in to change notification settings - Fork 231
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
Make lock dir configurable #435
Conversation
PR ppp-project#435 makes lockdir default go back to /var/lock, but runtime dir still changed from /run to /run/pppd in commit 66a8c74 ("Let ./configure control the paths for pppd") and is likely to not exist on some distros, in which case the pppdb will not be created. This is not a big problem but might as well just try to create the directory if it is missing. Return code of mkdir does not need to be checked as the following open will fail anyway if mkdir failed. See: ppp-project#419 Signed-off-by: Dominique Martinet <[email protected]>
(ah, note the default for localstatedir is /usr/local/var so that'll make the default lock dir /usr/local/var/lock and not /var/lock, but distros will configure |
fixed make check (had to add |
@paulusmack: @enaess has done some comments in the other PR recently. |
I do not think that the locks directory should be configurable: |
Happy to hard-code it back to /var/lock then, I can update this PR ~end of next week if that's preferred; that should simplify this patch quite a bit. |
Sorry for the delay I had forgotten to update this to hardcode back the lock path. I know the standard path is /var/lock, but since we've always been using /var/run/lock I've preferred to revert back to the old path instead of using /var/lock: happy to change it to /var/lock, there's just no standard define for it in paths.h Also, my original commit had a drive-by fix: "the else cases were incorrect for runtime dir and log dirs, and have been fixed" |
Actually, in ppp-2.4.9 and prior, the lock directory is /var/lock on Linux or /var/spool/locks on Solaris. The lock directory changed in commit 66a8c74 ("Let ./configure control the paths for pppd", 2022-07-30) to PPP_PATH_LOCKDIR set in pathnames.h to /var/run/lock. On the whole I would rather we went back to using /var/lock. |
lock dir changed on linux from /var/lock to /run/pppd/lock with pppd-2.5.0, which makes pppd fail to start if the distribution does not pre-create the directory. This reverts it back to /var/lock. The paths for other OS should be identical as LOCALSTATEDIR should be /var, but also revert them back as well just in case. Since the variable is no longer used remove it from makefiles. Fixes: 66a8c74 ("Let ./configure control the paths for pppd") Fixes: ppp-project#419 Signed-off-by: Dominique Martinet <[email protected]>
Oh, sorry I didn't go far enough. That also made me notice the non-standard LOCALSTATEDIR define we use again which should indeed be /var, but since it looks like the general consensus is to not make it configurable I've reverted back to explicit /var for all OSes and removed the now-unused variable. (Happy to use LOCALSTATEDIR instead if that's better, just say) |
Thanks! |
Hmm, sorry for the double-notification to everyone following this, but just a head's up github did something a bit weird here: it apparently rewrote the from of the merged commit? martinetd@f838224 (original commit that I submitted/was reviewed) had From with my work e-mail (@atmark-techno.com) and doesn't have a co-authored-by; (I'm not expecting this to be fixed at this point, but that also means the commit doesn't have a sign off from its "author") I've never seen this before so not sure what repo setting is involved here, but someone might want to have a look... (EDIT: I've now added work email to this github account so my other PR should work fine, but this is still annoying...) |
Backport upstream fix for pppd failing to start when lock dir does not exist (by reverting the lock dir to somewhere that should always exist) This fixes using pppd from something else than the main pppd service (e.g. networkmanager of ifupdown-ng), as these would not go through the checkpath hook, and pppd can now run without /run/ppp Link: ppp-project/ppp#435 Fixes: #15145
Backport upstream fix for pppd failing to start when lock dir does not exist (by reverting the lock dir to somewhere that should always exist) This fixes using pppd from something else than the main pppd service (e.g. networkmanager of ifupdown-ng), as these would not go through the checkpath hook, and pppd can now run without /run/ppp (cherry picked from commit 4a61d71) Link: ppp-project/ppp#435 Fixes: #15145
[ commit 4a61d7196b8c5808cad5fbed61aba06934bd256f ] Backport upstream fix for pppd failing to start when lock dir does not exist (by reverting the lock dir to somewhere that should always exist) This fixes using pppd from something else than the main pppd service (e.g. networkmanager of ifupdown-ng), as these would not go through the checkpath hook, and pppd can now run without /run/ppp Link: ppp-project/ppp#435 Fixes: #15145
lock dir changed on linux from /var/lock to /run/pppd/lock with pppd-2.5.0, which makes pppd fail to start if the distribution does not pre-create the directory.
This reverts it back to /var/lock.
In the line of configurability, also make the lock dir configurable through ./configure --with-lock-dir for distributions that prefer /run/pppd/lock, but change the default for linux back to /var/lockThis commit is bigger than it should be, because the current configure mechanism for PPPD_RUNTIME_DIR and others is inconditional: pathnames.h has #else cases for when the variable is not defined but that is not possible.This does not work for lock dir, because on non-linux platform the path is different, and reproducing this logic at configure time is not trivial.Instead of the current method, only set the the PPPD_X_DIR variables in pppdconf.h if it was explicitly set in configure, and otherwise leave it to the else case.Note:the else cases were incorrect for runtime dir and log dirs, and have been fixedPPPD_PLUGIN_DIR has been kept as is because it is used for rpath in makefiles and needs to be set inconditionnaly at configure timeFixes: 66a8c74 ("Let ./configure control the paths for pppd")
Fixes: #419