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

Victor Lowther victor.lowther at gmail.com
Mon Feb 4 10:11:19 PST 2008


On Mon, 2008-02-04 at 08:48 -0800, Dan Nicholson wrote:
> 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.

Well, if we don't export PM_FUNCTIONS in pm-action and pm-powersave, we
would have to export it in functions, and that seemed a bit weirdly
recursive to me.

Everything is done in process?  The hooks, at least, are separate
processes.

> 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.

It sounds like a good idea -- I did not think of collecting all the
relocatable stuff at the top of functions, I just did a global search
and replace in vim.

The version I end up pushing upstream will have this change.

> 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.

Because (oddly enough), automake thinks of scripts as data that happens
to be executable, not as executable files. I suppose since you normally
don't compile scripts...

I ran into this issue with my first attempt at making the scripts
relocatable -- the make process would attempt to rewrite the scripts
before I had moved them into place, even though having the rewrite rule
in install-exec-hook should have guaranteed that the scripts were in
their final location.

> Looks good besides those nits, though.

Thanks!
-- 
Victor Lowther
Ubuntu Certified Professional


More information about the Pm-utils mailing list