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.

Sign up for GitHub

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

Build: configure: add --with-runstatedir option #136

Merged
merged 3 commits into from
Oct 14, 2021
Merged

Build: configure: add --with-runstatedir option #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e1e0c0
Build: configure: add --with-runstatedir option
gao-yan Oct 12, 2021
5067f83
Doc: man: refer to the modern run state directory `/run` if appropriate
gao-yan Oct 13, 2021
515bae0
Build: configure: validate configure options for paths
kgaillot Oct 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 57 additions & 23 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,34 @@ m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([no])])
AM_INIT_AUTOMAKE(1.11.1 foreign TESTS_OPTION)
AM_PROG_CC_C_O

# expand_path_option $path_variable_name $default
expand_path_option() {
# The first argument is the variable *name* (not value)
ac_path_varname="$1"

# Get the original value of the variable
ac_path_value=$(eval echo "\${${ac_path_varname}}")

# Expand any literal variable expressions in the value so that we don't
# end up with something like '${prefix}' in #defines etc.
#
# Autoconf deliberately leaves values unexpanded to allow overriding
# the configure script choices in make commands (for example,
# "make exec_prefix=/foo install"). No longer being able to do this seems
# like no great loss.
eval ac_path_value=$(eval echo "${ac_path_value}")

# Use (expanded) default if necessary
AS_IF([test x"${ac_path_value}" = x""],
[eval ac_path_value=$(eval echo "$2")])

# Require a full path
AS_CASE(["$ac_path_value"],
[/*], [eval ${ac_path_varname}="$ac_path_value"],
[*], [AC_MSG_ERROR([$ac_path_varname value "$ac_path_value" is not a full path])]
)
}

PKG_CHECK_MODULES(glib, [glib-2.0])
PKG_CHECK_MODULES(libxml, [libxml-2.0])

Expand Down Expand Up @@ -164,6 +192,15 @@ AC_ARG_WITH(configdir,
[ CONFIGDIR="$withval" ]
)

dnl --runstatedir is available as of autoconf 2.70 (2020-12-08). When users
dnl have an older version, they can use our --with-runstatedir.
sbd_runstatedir=""
AC_ARG_WITH([runstatedir],
[AS_HELP_STRING([--with-runstatedir=DIR],
[modifiable per-process data @<:@LOCALSTATEDIR/run@:>@ (ignored if --runstatedir is available)])],
[ sbd_runstatedir="$withval" ]
)

SBD_WATCHDOG_TIMEOUT_DEFAULT=""
AC_ARG_WITH(watchdog-timeout-default,
[ --with-watchdog-timeout-default=SECONDS
Expand Down Expand Up @@ -289,32 +326,29 @@ case $exec_prefix in
prefix) exec_prefix=$prefix;;
esac

dnl Expand autoconf variables so that we dont end up with '${prefix}'
dnl in #defines and python scripts
dnl NOTE: Autoconf deliberately leaves them unexpanded to allow
dnl make exec_prefix=/foo install
dnl No longer being able to do this seems like no great loss to me...

eval prefix="`eval echo ${prefix}`"
eval exec_prefix="`eval echo ${exec_prefix}`"
eval bindir="`eval echo ${bindir}`"
eval sbindir="`eval echo ${sbindir}`"
eval libexecdir="`eval echo ${libexecdir}`"
eval datadir="`eval echo ${datadir}`"
eval sysconfdir="`eval echo ${sysconfdir}`"
eval sharedstatedir="`eval echo ${sharedstatedir}`"
eval localstatedir="`eval echo ${localstatedir}`"
eval libdir="`eval echo ${libdir}`"
eval includedir="`eval echo ${includedir}`"
eval oldincludedir="`eval echo ${oldincludedir}`"
eval infodir="`eval echo ${infodir}`"
eval mandir="`eval echo ${mandir}`"
dnl Expand values of autoconf-provided directory options
expand_path_option prefix
expand_path_option exec_prefix
expand_path_option bindir
expand_path_option sbindir
expand_path_option libexecdir
expand_path_option datadir
expand_path_option sysconfdir
expand_path_option sharedstatedir
expand_path_option localstatedir
expand_path_option libdir
expand_path_option includedir
expand_path_option oldincludedir
expand_path_option infodir
expand_path_option mandir

AS_IF([test x"${runstatedir}" = x""], [runstatedir="${sbd_runstatedir}"])
Copy link

Choose a reason for hiding this comment

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

didn't think of this when reviewing the change in pacemaker but that could actually be further optimized
with expand_path_option having a longer list of parameters at the end where it takes the first !="".
might loose portability though as I didn't find M4sh macros that expand into a loop and shift is a bashism ...
anyway maybe something to note down for next iteration ...

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it'd make expand_path_option() a little complicated/messy ... runstatedir seems kind of treated as a special case here about how the values might come from different sources ...

Copy link

Choose a reason for hiding this comment

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

As said if M4sh had something like AS_WHILE and some abstraction for shift the change would be insignificant and having various sources with an order doesn't sound if it might not be useful for other things on the longer run.

expand_path_option runstatedir "${localstatedir}/run"
AC_SUBST(runstatedir)

Copy link

Choose a reason for hiding this comment

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

Did you see the expand_path_option thing Ken has just done in pacemaker?
We should as well consider if we want it synced with .../tests/configure.ac - better of course pull straight so that the copy isn't needed - had some issue i don't recall in detail for which this was a quick hack that persisted.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you see the expand_path_option thing Ken has just done in pacemaker?

Alright, noticed it. We can add the same as well.

We should as well consider if we want it synced with .../tests/configure.ac - better of course pull straight so that the copy isn't needed - had some issue i don't recall in detail for which this was a quick hack that persisted.

OK, will add commits to get this synced with that one when everything is fine.

Copy link

Choose a reason for hiding this comment

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

yep - will look over it after this PR and try to remember ... in the worst case we can still simply sync ...

AC_SUBST(LIBADD_DL) dnl extra flags for dynamic linking libraries

if test x"${CONFIGDIR}" = x""; then
CONFIGDIR="${sysconfdir}/sysconfig"
fi
expand_path_option CONFIGDIR "${sysconfdir}/sysconfig"
AC_SUBST(CONFIGDIR)

if test x"${SBD_WATCHDOG_TIMEOUT_DEFAULT}" = x""; then
Expand Down
2 changes: 1 addition & 1 deletion man/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ sbd.sysconfig.pod: ../src/sbd.sysconfig
sed -r -n -e "s/^## Type: (.*)/Allows C<\1>/;t type;s/^## Default: (.*)/ defaulting to C<\1>/;t default;s/^#*(.*)=.*/=item B<\1>\n/;t variable;s/^#*//;s/^ *//;H;d;:type;h;d;:default;H;x;s/\n//;x;d;:variable;G;p" $< > $@

sbd.8.pod: sbd.8.pod.in sbd.sysconfig.pod
sed -e "s/@environment_section@//;t insert;p;d;:insert;rsbd.sysconfig.pod" $< > $@
sed -e "s,\@runstatedir\@,$(runstatedir)," $< |sed -e "s/@environment_section@//;t insert;p;d;:insert;rsbd.sysconfig.pod" > $@

sbd.8: sbd.8.pod
@POD2MAN@ -s 8 -c "STONITH Block Device" -r "SBD" -n "SBD" $< $@
Expand Down
2 changes: 1 addition & 1 deletion man/sbd.8.pod.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ Defaults to I<enabled>.
This can be used to override the default watchdog device used and should not
usually be necessary.

=item B<-p> F</var/run/sbd.pid>
=item B<-p> F<@runstatedir@/sbd.pid>

Copy link

@wenningerk wenningerk Oct 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If we put something hacky like

sbd.8.pod: sbd.8.pod.in sbd.sysconfig.pod
	sed -e "s,@runstatedir@,$(runstatedir)," $< |sed -e "s/@environment_section@//;t insert;p;d;:insert;rsbd.sysconfig.pod" > $@

in man/Makefile.am we could even make it dynamic ;-)

Copy link

Choose a reason for hiding this comment

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

Atm I have at least no good idea how to use the autoconf-config-file capabilities here :-(
having environment_section=$(shell cat sbd.sysconfig.pod) probably won't work as we can't get that in as prerequisite.
But maybe with the sed-code that creates sbd.sysconfig.pod ... depends on when/if the vars are evaluated ...

Copy link
Member Author

@gao-yan gao-yan Oct 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I haven't figured out a better idea either. It's kind of tricky how any recognized variables are expanded by automake here, but it at least seems to work this way:

       sed -e "s,\@runstatedir\@,$(runstatedir)," $< |sed -e "s/@environment_section@//;t insert;p;d;:insert;rsbd.sysconfig.pod" > $@

This option can be used to specify a pidfile for the main sbd process.

Expand Down
3 changes: 2 additions & 1 deletion sbd.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ regression-testing sbd.
export CFLAGS="$RPM_OPT_FLAGS -Wall -Werror"
%configure --with-watchdog-timeout-default=%{watchdog_timeout_default} \
--with-sync-resource-startup-default=%{?with_sync_resource_startup_default:yes}%{!?with_sync_resource_startup_default:no} \
--with-sync-resource-startup-sysconfig=%{sync_resource_startup_sysconfig}
--with-sync-resource-startup-sysconfig=%{sync_resource_startup_sysconfig} \
--with-runstatedir=%{_rundir}
make %{?_smp_mflags}
###########################################################

Expand Down
4 changes: 2 additions & 2 deletions src/sbd.service.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ RefuseManualStart=true

[Service]
Type=forking
PIDFile=@localstatedir@/run/sbd.pid
PIDFile=@runstatedir@/sbd.pid
EnvironmentFile=-@CONFIGDIR@/sbd
ExecStart=@sbindir@/sbd $SBD_OPTS -p @localstatedir@/run/sbd.pid watch
ExecStart=@sbindir@/sbd $SBD_OPTS -p @runstatedir@/sbd.pid watch
ExecStop=@bindir@/kill -TERM $MAINPID

# Could this benefit from exit codes for restart?
Expand Down
2 changes: 1 addition & 1 deletion src/sbd.sh.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ fi
SBD_DEVS=${SBD_DEVICE%;}
SBD_DEVICE_ARGS="-d ${SBD_DEVS//;/ -d }"

: ${SBD_PIDFILE:=/var/run/sbd.pid}
: ${SBD_PIDFILE:=@runstatedir@/sbd.pid}
SBD_OPTS+=" -p $SBD_PIDFILE"
: ${SBD_PACEMAKER:="true"}
if ocf_is_true "$SBD_PACEMAKER" ; then
Expand Down
4 changes: 2 additions & 2 deletions src/sbd_remote.service.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ RefuseManualStart=true

[Service]
Type=forking
PIDFile=@localstatedir@/run/sbd.pid
PIDFile=@runstatedir@/sbd.pid
EnvironmentFile=-@CONFIGDIR@/sbd
ExecStart=@sbindir@/sbd $SBD_OPTS -p @localstatedir@/run/sbd.pid watch
ExecStart=@sbindir@/sbd $SBD_OPTS -p @runstatedir@/sbd.pid watch
ExecStop=@bindir@/kill -TERM $MAINPID

# Could this benefit from exit codes for restart?
Expand Down