[Pm-utils] [patch commit] Hook and install location independence, the less ugly version

[Dan Nicholson]

On Feb 2, 2008 11:21 AM, Victor Lowther <victor.lowther at gmail.com> wrote:
> This patchset just adds simple location independence.  It can also be
> pulled from git://fnordovax.org/pm-utils/ (branch named
> vlowther-simple-location-independence)
>
> It uses automake to transform src/pm-action.in, src/pm-powersave.in,
> and pm/functions.in into src/pm-action, src/pm-powersave, and
> pm/functions respectivly.  See the appropriate Makefile.am files for
> the details.
>
> Location independence of the hooks is achieved by having pm-action and
> pm-powersave export PM_FUNCTIONS, which points to the installed
> location of pm/functions.
> The hooks have been modified to source functions from that environment variable.
>
> Comments?

It would be really great if you could inline the patches. That makes
reviewing much easier since the comments can be right in the patch.
git-send-email can do this, but the UI is rough. Here's my
git-send-email cheat sheet:

Single patches:
git-format-patch -o <patchdir> HEAD^
git send-email --to <list> --no-signed-off-by-cc \
        --suppress-from --no-thread \
        <patchdir>

Remove --no-thread to be prompted for Message-ID

Patch series from branch:
git diff --stat --summary master.. > mailmsg
git format-patch -n -o <patchdir> master
git send-email --to <list> --no-signed-off-by-cc --suppress-from \
        --compose --no-chain-reply-to \
        --subject "[PATCH 0/x] <patch subject>" <patchdir>
<include + edit mailmsg>

Now to the review. I took a quick glance, but probably need to look again.

0001: Is there reason to export PM_FUNCTIONS in pm-action? I believe
everything is done in process.

0006: This could come later, but I think it would be nicer to collect
all the relocatable directories into variables at the top of the
script instead of substituting @PM_*@ throughout the script. Like:

PM_UTILS_LIBDIR=@PM_UTILS_LIBDIR@
PM_UTILS_SYSCONFDIR=@PM_UTILS_SYSCONFDIR@
...
for cfg in "$PM_UTILS_SYSCONFDIR"/config.d/*[!~] ; do

That would pave the way where you could override things at runtime for
testing or whatnot. Maybe it's not worth the trouble.

0007: Any reason to change this from install-exec-hook to
install-data-hook? They're links to scripts, so they should be
considered part of the executables, I think.

Looks good besides those nits, though.