Ryan Harper has posted comments on this change.
Change subject: make compression type configurable, support gzip and xz
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(5 inline comments)
Please review the comments in configure.ac; let's make it much simpler by not exposing
a --with-xx option, but a much simpler check for xz, unxz, gzip, gunzip, and then based on
whether xz is found, set the compression type. If we find xz, use it, if not, use
gzip/gunzip.
....................................................
File configure.ac
Line 40: AM_INIT_AUTOMAKE([-Wno-portability])
Line 41:
Line 42: # the make dist compress type
Line 43: AC_PATH_PROG([XZ], [xz])
Line 44: AC_PATH_PROG([GZIP], [gzip])
These checks should be down in the sorted-list of AC_PATH_PROG checks (line 124).
Line 45: if test "x$XZ" = "x"; then
Line 46: if test "x$GZIP" = "x"; then
Line 47: AC_MSG_ERROR([neither xz and gzip is found, please install one of them])
Line 48: else
Line 46: if test "x$GZIP" = "x"; then
Line 47: AC_MSG_ERROR([neither xz and gzip is found, please install one of them])
Line 48: else
Line 49: AC_SUBST([DIST_TYPE_SUFFIX], [gz])
Line 50: AC_MSG_NOTICE([support gzip compress])
Configuration selection is usually emitted after all of the checks have been completed. I
would move all of the logic around selecting the compression type after all of the PATH
checking is complete.
Line 51: fi
Line 52: else
Line 53: AC_SUBST([DIST_TYPE_SUFFIX], [xz])
Line 54: AC_MSG_NOTICE([support xz compress])
Line 67: [logrotate_compress=gzip],
Line 68: [logrotate_compress=xz]
Line 69: )
Line 70: ]
Line 71: )
I don't think this needs to be surfaced as a configure switch. Rather we simply want
to prefer xz over gzip if it's present. If we have xz, then we can set the
compression type to that and use it.
If we don't find xz and we don't find gzip, we can error out indicating we need
some sort of compression program.
Line 72: AC_MSG_NOTICE([configure logrotate compress by $logrotate_compress])
Line 73:
Line 74: if test "x$logrotate_compress" = xgzip; then
Line 75: AC_SUBST([COMPRESS_SUFFIX], [gz])
Line 74: if test "x$logrotate_compress" = xgzip; then
Line 75: AC_SUBST([COMPRESS_SUFFIX], [gz])
Line 76: AC_SUBST([REQUIRE_COMPRESS], [gzip])
Line 77: AC_PATH_PROG([COMPRESSCMD], [gzip], [/usr/bin/gzip])
Line 78: AC_PATH_PROG([UNCOMPRESSCMD], [gunzip], [/usr/bin/gunzip])
Just add the uncompress commands to the list of external programs to check; we don't
need to optionally check.
Line 79: else
Line 80: AC_SUBST([COMPRESS_SUFFIX], [xz])
Line 81: AC_SUBST([REQUIRE_COMPRESS], [xz])
Line 82: AC_PATH_PROG([COMPRESSCMD], [xz], [/usr/bin/xz])
Line 82: AC_PATH_PROG([COMPRESSCMD], [xz], [/usr/bin/xz])
Line 83: AC_PATH_PROG([UNCOMPRESSCMD], [unxz], [/usr/bin/unxz])
Line 84: fi
Line 85: AC_CHECK_FILE(vdsm.spec, [rm -f vdsm.spec])
Line 86: AC_CHECK_FILE(vdsm/vdsm-logrotate.conf, [rm -f vdsm/vdsm-logrotate.conf])
Do not delete the generated files that you don't create; This is handled by make clean
or distclean targets.
Line 87:
Line 88: # Checking for build tools
Line 89: AC_PROG_CC
Line 90: AC_PROG_LN_S
--
To view, visit
http://gerrit.ovirt.org/4174
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d88e9d40aefc87564c9c7e23f0d28aaad867aca
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server