[PATCHES] CTDB - fix script and documentation paths at build/install time

Michael Adam obnox at samba.org
Mon Sep 14 21:57:37 UTC 2015


Great cleanup work!
Pushed to autobuild.

Additional review by José included.

Cheers - Michael

On 2015-09-11 at 17:39 +1000, Martin Schwenke wrote:
> On Fri, 11 Sep 2015 17:19:28 +1000, Amitay Isaacs <amitay at gmail.com>
> wrote:
> 
> > On Fri, Sep 11, 2015 at 2:10 PM, Martin Schwenke <martin at meltin.net> wrote:
> > 
> > > On Thu, 10 Sep 2015 20:18:58 +1000, Martin Schwenke <martin at meltin.net>
> > > wrote:
> > >
> > > > On Tue, 8 Sep 2015 19:55:31 +1000, Martin Schwenke <martin at meltin.net>
> > > > wrote:
> > > >
> > > > > [...]
> > > >
> > > > > Currently CTDB's scripts and documentation aren't consistent with
> > > > > build/install-time settings for various paths.  This set of patches
> > > > > attempts to fix that.  The first few patches are clean-ups and the
> > > > > rest make sure everything is set nicely at build/install time.
> > > > >
> > > > > These patches are in my ctdb-script-paths branch:
> > > > >
> > > > >
> > > https://git.samba.org/?p=martins/samba.git;a=shortlog;h=refs/heads/ctdb-script-paths
> > > > >
> > > > >   git://git.samba.org/martins/samba.git (ctdb-script-paths branch)
> > > > >
> > > > > Review and push appreciated.  Thanks!
> > > >
> > > > I've updated the branch above.  I won't clutter the list by reposting...
> > > >
> > > > Changes from previous patchset are based on a partial review by Amitay.
> > > >
> > > > Changes are:
> > > >
> > > > * Tweak the wscript changes so variable names are simpler and are more
> > > >   consistent with the way things are already done.
> > > >
> > > > * Drop this patch:
> > > >
> > > >     ctdb-scripts: Prefer configuration and initscripts in SYSCONFDIR
> > > >
> > > >   My main concern here was to make sure the CTDB configuration can be
> > > >   loaded regardless of installation prefix.  However, this is already
> > > >   OK.  If the prefix is /usr/local then the configuration can live
> > > >   in /usr/local/etc/ctdb/ctdbd.conf.  This works fine.
> > > >
> > > >   This patch also tried to address ability to load configuration for
> > > >   other subsystems (e.g. NFS) that aren't installed as part of a
> > > >   distribution.  It didn't do a good job of that so I'm happy to make
> > > >   another attempt at this later.
> > > >
> > > >   The patch also had a bug.  :-)
> > > >
> > > > Michael, I understand you'd like to review this.  If you're happy with
> > > > it then please post a "Reviewed-by:" rather than pushing.  I'd also
> > > > like to give Amitay a chance to comment on the last few patches that he
> > > > didn't get to yesterday...  :-)
> > > >
> > > > I've tested this with the default prefix of /usr/local and starting
> > > > CTDB with ctdbd_wrapper, as well as building RPMs using CTDB's
> > > > packaging support.  I still need to test what happens in the case where
> > > > xsltproc isn't available and perhaps make a minor change to the manpage
> > > > handling.
> > >
> > > Re-pushed my branch again with 2 fairly minor changes:
> > >
> > > * ctdb-scripts: Properly set CTDB_VARDIR in scripts at install time
> > >
> > >   Use os.path.join("doc", x") instead of "doc/%s" % (x) in 1 place
> > >
> > > * Add a patch to install pre-built manpages if xsltproc is not
> > >   installed.
> > >
> > >   The manpages were never installed if xsltproc is not installed.
> > >   Happily, the rest of the patchset doesn't introduce a new error.
> > >
> > >   I'm happy to drop this if it requires more discussion (about whether
> > >   or not to filter the paths in the pre-built manpages), since it is
> > >   just an extra bug fix.  However, it seems like a good thing to
> > >   include.  :-)
>  
> > The patches look fine with two suggested changes.
> > 
> > Martin will push the latest version of his tree or send patches to the list.
> > 
> > Reviewed-by: me.
> > 
> > Amitay.
> 
> The 2 changes are:
> 
> * Clarify this commit message (which was previously confusing):
> 
>   [PATCH 11/16] ctdb-scripts: CTDB_BASE must be set when including
>   functions file
> 
> * Dropped flat=True from the new INSTALL_FILES() in the last patch,
>   which is new anyway...
> 
> Final version is attached...
> 
> peace & happiness,
> martin


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150914/5e2e9250/attachment.sig>


More information about the samba-technical mailing list