[lxc-devel] [PATCH 3/3] lxc-rpm-build

[Dwight Engen]

On Mon, 10 Sep 2012 13:53:50 -0400
Stéphane Graber <stgraber at ubuntu.com> wrote:

[...]
> >>> diff --git a/configure.ac b/configure.ac
> >>> index 70c74ec..b8d86e6 100644
> >>> --- a/configure.ac
> >>> +++ b/configure.ac
> >>> @@ -70,11 +70,11 @@ AC_ARG_WITH([rootfs-path],
> >>>  	)], [], [with_rootfs_path=['${libdir}/lxc/rootfs']])
> >>>  
> >>>  AS_AC_EXPAND(LXC_GENERATE_DATE, "$(date)")
> >>> +AS_AC_EXPAND(LXCPATH, "${with_config_path}")
> >>> +AS_AC_EXPAND(LXCROOTFSMOUNT, "${with_rootfs_path}")
> >>> +AS_AC_EXPAND(LXCTEMPLATEDIR, ["${datadir}/lxc/templates"])
> >>>  
> >>> -AC_SUBST(LXCPATH, "${with_config_path}")
> >>> -AC_SUBST(LXCROOTFSMOUNT, "${with_rootfs_path}")
> >>>  AC_SUBST(LXCINITDIR, ['${libexecdir}'])
> >>> -AC_SUBST(LXCTEMPLATEDIR, ['/usr/share/lxc/templates'])
> >>
> >> This might require some matching changes to the templates and other
> >> files using these variables as I believe some workarounds were put
> >> in place to deal with the unexpanded variables, but I still think
> >> it's a good thing and should help cleanup some code.
> > 
> > Could you point me to what I need to fixup? The only templates my
> > grep found were lxc-altlinux.in and lxc-sshd.in and the uses there
> > (of LXCPATH and LXCTEMPLATEDIR respectively) look okay to me when
> > expanded vs subst.
> > 
> > The reason I changed them to expand was so the docs would be right
> > (they had SUBST paths with ${var} still in them) but obviously I
> > don't want to break code just to make the docs right :)
> 
> I'm not actually expending breakage but I'm expecting some dead code.
> For example @LXCPATH@ is used in most scripts using some workarounds
> as the value wasn't expanded, like:
> libdir=@LIBDIR@
> libexecdir=@LIBEXECDIR@
> localstatedir=@LOCALSTATEDIR@
> 
> Once we apply your change, we'd have to check for such dead code and
> clean it up.

Ahh yes, I've seen this workaround code. I'll look into removing it in
the patch as well.

[...]
> >> Why the --disable-apparmor? I believe the apparmor option has the
> >> required checks in the configure to only build if you have the
> >> required libraries/dev packages installed.
> > 
> > The configure check didn't work for me, let me look into why as that
> > is the right way to do it, it should "just work".
> 
> Thanks. If the configure check is wrong, I'd rather have this fixed
> rather than working around it in the packaging.

I've confirmed that the configure check works fine in whats in staging
so there is no need for this in the .spec file.