On 12-03-12 03:12 PM, Jakob Bornecrantz wrote: > ----- Original Message ----- >> Use AM_CONDITIONAL. Automake knows what to distribute. >> It needs to be able to navigate down the subdirs to find what >> needs to be included in the tarball. >> >> To test reliably, create a tarball and expand it into a separate >> directory and build with xatracker. Distcheck will not detect >> missing code when such code is configured not to build. >> >> The content of a tarball *must* always be identical, regardless >> of the configuration options used or on which platform it was >> configured. >> >> Signed-off-by: Gaetan Nadon <[email protected]> > Thanks, I had just done a simpler patch, but less correct then > yours, so I'll rather use this. > > Do we really need to do the conditional in all the > Makefile.am's according to the link below that isn't needed > and it will figure out DIST_SUBDIRS correctly? No, not really. Like any coding job, there are different ways of doing it with different pros and cons. These are some of the experiences I had which influence my choices:
Developers often end-up in a Makefile without having gone through configure.ac, hence they don't know statements in there that might affect the Makefile. They would not even notice that vmwglx is conditionally compiled. One may invoke make from the vmwglx subdir on a system that does not have the xatracker and waste time chasing ghost problems. It's confusing when 'make' does not behave the same when invoked from subdirs. The /src makefile still needs a conditional. It can be done through configure.ac but that increases this file complexity. I can see cases where a conditional SUBDIRS would be preferable. Over time you get used to do it one way over the other. There is really nothing wrong, feel free to change it. BTW, saa can always be built, right? That would help finding compiler errors and be subject to distcheck verification. That would leave us with just the vmwglx subdir to be conditionally skipped. Also, if you feel like it, AC_SYS_LARGEFILE does not really need to be in a condition. It's harmless and can go with other AC_ statements at the top in the "Initialize Autoconf" section. Thanks! http://www.gnu.org/software/automake/manual/html_node/Subdirectories-with-AM_005fCONDITIONAL.html Cheers, Jakob. >> --- >> Makefile.am | 4 +++- >> configure.ac | 18 ++++++------------ >> saa/Makefile.am | 4 +++- >> src/Makefile.am | 7 +++++-- >> vmwgfx/Makefile.am | 6 +++--- >> 5 files changed, 20 insertions(+), 19 deletions(-) >> >> diff --git a/Makefile.am b/Makefile.am >> index 1203715..64c019e 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -18,7 +18,9 @@ >> # IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF >> OR IN >> # CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE >> SOFTWARE. >> >> -SUBDIRS = @VMWGFX_DIRS@ src man vmwarectrl >> +# Order: vmwgfx before src >> +SUBDIRS = man saa vmwgfx src vmwarectrl >> + >> MAINTAINERCLEANFILES = ChangeLog INSTALL >> .PHONY: ChangeLog INSTALL >> >> diff --git a/configure.ac b/configure.ac >> index cf1491f..af2737a 100644 >> --- a/configure.ac >> +++ b/configure.ac >> @@ -120,29 +120,23 @@ DRIVER_NAME=vmware >> AC_SUBST([DRIVER_NAME]) >> >> AC_MSG_CHECKING([whether to build Kernel Mode Setting and 3D]) >> -VMWGFX_DIRS= >> if test x$BUILD_VMWGFX = xyes; then >> AC_MSG_RESULT([yes]) >> AC_SYS_LARGEFILE >> - VMWGFX_DIRS="saa vmwgfx" >> - VMWGFX_LIBADD='$(top_builddir)/vmwgfx/libvmwgfx.la' >> - AC_CONFIG_FILES([ >> - saa/Makefile >> - vmwgfx/Makefile >> - ]) >> - AC_DEFINE([BUILD_VMWGFX], 1, >> - [Building the vmwgfx driver path]) >> + AC_DEFINE([BUILD_VMWGFX], 1, [Building the vmwgfx driver >> path]) >> else >> AC_MSG_RESULT([no]) >> fi >> >> -AC_SUBST([VMWGFX_DIRS]) >> -AC_SUBST([VMWGFX_LIBADD]) >> +AM_CONDITIONAL(BUILD_VMWGFX, test "x$BUILD_VMWGFX" = xyes) >> + >> AC_CONFIG_FILES([ >> Makefile >> + man/Makefile >> + saa/Makefile >> + vmwgfx/Makefile >> src/Makefile >> vmwarectrl/Makefile >> - man/Makefile >> ]) >> >> AC_OUTPUT >> diff --git a/saa/Makefile.am b/saa/Makefile.am >> index 849ced9..48c9734 100644 >> --- a/saa/Makefile.am >> +++ b/saa/Makefile.am >> @@ -1,3 +1,5 @@ >> + >> +if BUILD_VMWGFX >> noinst_LTLIBRARIES = libsaa.la >> >> libsaa_la_CFLAGS = $(CWARNFLAGS) $(XORG_CFLAGS) >> @@ -10,4 +12,4 @@ libsaa_la_SOURCES = \ >> saa_render.c \ >> saa_accel.c \ >> saa.h >> - >> +endif >> diff --git a/src/Makefile.am b/src/Makefile.am >> index 1f54168..04c9e0d 100644 >> --- a/src/Makefile.am >> +++ b/src/Makefile.am >> @@ -28,8 +28,11 @@ vmware_drv_la_LTLIBRARIES = vmware_drv.la >> vmware_drv_la_LDFLAGS = -module -avoid-version >> vmware_drv_la_CFLAGS = $(CWARNFLAGS) @XORG_CFLAGS@ >> vmware_drv_ladir = @moduledir@/drivers >> -vmware_drv_la_LIBADD = @VMWGFX_LIBADD@ >> -vmware_drv_la_DEPENDENCIES = @VMWGFX_LIBADD@ >> + >> +if BUILD_VMWGFX >> +vmware_drv_la_LIBADD = $(top_builddir)/vmwgfx/libvmwgfx.la >> +vmware_drv_la_DEPENDENCIES = $(top_builddir)/vmwgfx/libvmwgfx.la >> +endif >> >> vmware_drv_la_SOURCES = \ >> bits2pixels.c \ >> diff --git a/vmwgfx/Makefile.am b/vmwgfx/Makefile.am >> index 813f1a2..269d870 100644 >> --- a/vmwgfx/Makefile.am >> +++ b/vmwgfx/Makefile.am >> @@ -1,3 +1,5 @@ >> + >> +if BUILD_VMWGFX >> noinst_LTLIBRARIES = libvmwgfx.la >> libvmwgfx_la_CFLAGS = $(CWARNFLAGS) $(XORG_CFLAGS) @LIBDRM_CFLAGS@ >> @XATRACKER_CFLAGS@ -I$(top_srcdir)/src -I$(top_srcdir)/saa >> libvmwgfx_la_LIBADD = @LIBDRM_LIBS@ $(top_builddir)/saa/libsaa.la\ >> @@ -24,6 +26,4 @@ libvmwgfx_la_SOURCES = \ >> vmwgfx_xa_composite.c \ >> vmwgfx_xa_surface.c \ >> wsbm_util.h >> - >> - >> - >> +endif >> -- >> 1.7.5.4 >> >> _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
