On 31 January 2017 at 21:05, Gaetan Nadon <[email protected]> wrote: > On 01/31/2017 05:26 AM, Emil Velikov wrote: >> >> From: Emil Velikov <[email protected]> >> >> Redirect stderr from git, sh and cp to /dev/null to cut down the 'spam' >> and reword the error messages to be less misleading. >> >> Why ? Currently CHANGELOG_CMD can fail for any of the following reasons: >> - git executable is missing >> - .git is missing and/or malformed >> - srcdir is RO - thus the stdout redirection will fail >> >> Yet things fail (yes that's fine) due to #3 on each `make distcheck' >> although #2 is the one suggested, always. >> >> If we really want to, we can attribute each case with respective >> message. Yet that will make the macro twice as large. Considering that >> we're unlikely to bother [or be able to do anything], keep things short >> and simple. >> >> Without the patch we get the following on each `make distcheck': >> >> " >> /bin/sh: ../../.changelog.tmp: Permission denied >> git directory not found: installing possibly empty changelog. >> cp: cannot create regular file '../../.INSTALL.tmp': Permission denied >> util-macros "pkgdatadir" from xorg-macros.pc not found: installing >> possibly empty INSTALL. >> /bin/sh: ../../.changelog.tmp: Permission denied >> git directory not found: installing possibly empty changelog. >> cp: cannot create regular file '../../.INSTALL.tmp': Permission denied >> util-macros "pkgdatadir" from xorg-macros.pc not found: installing >> possibly empty INSTALL. >> " >> >> and after: >> >> " >> git failed to create chanelog: installing possibly empty changelog. >> failed to copy INSTALL from util-macros: installing possibly empty >> INSTALL. >> git failed to create chanelog: installing possibly empty changelog. >> failed to copy INSTALL from util-macros: installing possibly empty >> INSTALL. >> " > > Looks good >> >> Cc: Gaetan Nadon <[email protected]> >> Cc: Peter Hutterer <[email protected]> >> Signed-off-by: Emil Velikov <[email protected]> >> --- >> As mentioned before - we could do a test -f so establish if file is >> already there. This will remove the warnings all together when doing >> `make distcheck'. > > Not sure it a good idea to skip the step if the ChangeLog file is already > present. It needs to be up-to-date as the developers can make a last minute > fix or just realized he was missing a commit. Also when one runs "make > ChangeLog", it must be replaced. > I was talking about omitting the 'touch' and warning message (or maybe rewording the latter), when the file is present. Roughly like the following (untested) - either way it's something which can be addressed at any stage.
-|| (rm -f \$(top_srcdir)/.changelog.tmp; touch \$(top_srcdir)/ChangeLog; \ -echo 'git directory not found: installing possibly empty changelog.' >&2)" +|| (rm -f \$(top_srcdir)/.changelog.tmp; test -e \$(top_srcdir)/ChangeLog || ( \ +touch \$(top_srcdir)/ChangeLog; \ +echo 'git failed to create ChangeLog: installing empty ChangeLog.' >&2))" >> Please state your preference for/again each approach. >> --- >> xorg-macros.m4.in | 4 ++-- >> xorgversion.m4 | 4 ++-- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in >> index 2ed7837..70f6a75 100644 >> --- a/xorg-macros.m4.in >> +++ b/xorg-macros.m4.in >> @@ -1837,9 +1837,9 @@ m4_ifdef([AM_SILENT_RULES], >> [AM_SILENT_RULES([yes])], >> AC_DEFUN([XORG_INSTALL], [ >> AC_REQUIRE([PKG_PROG_PKG_CONFIG]) >> macros_datadir=`$PKG_CONFIG --print-errors --variable=pkgdatadir >> xorg-macros` >> -INSTALL_CMD="(cp -f "$macros_datadir/INSTALL" \$(top_srcdir)/.INSTALL.tmp >> && \ >> +INSTALL_CMD="(cp -f "$macros_datadir/INSTALL" \$(top_srcdir)/.INSTALL.tmp >> 2>/dev/null && \ >> mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \ >> || (rm -f \$(top_srcdir)/.INSTALL.tmp; touch \$(top_srcdir)/INSTALL; \ >> -echo 'util-macros \"pkgdatadir\" from xorg-macros.pc not found: >> installing possibly empty INSTALL.' >&2)" >> +echo 'failed to copy INSTALL from util-macros: installing possibly empty >> INSTALL.' >&2)" >> AC_SUBST([INSTALL_CMD]) >> ]) # XORG_INSTALL >> diff --git a/xorgversion.m4 b/xorgversion.m4 >> index 19f2ffd..d5c4377 100644 >> --- a/xorgversion.m4 >> +++ b/xorgversion.m4 >> @@ -56,9 +56,9 @@ AC_DEFUN([XORG_RELEASE_VERSION],[ >> # >> # >> AC_DEFUN([XORG_CHANGELOG], [ >> -CHANGELOG_CMD="(GIT_DIR=\$(top_srcdir)/.git git log > >> \$(top_srcdir)/.changelog.tmp && \ >> +CHANGELOG_CMD="((GIT_DIR=\$(top_srcdir)/.git git log > >> \$(top_srcdir)/.changelog.tmp) 2>/dev/null && \ >> mv \$(top_srcdir)/.changelog.tmp \$(top_srcdir)/ChangeLog) \ >> || (rm -f \$(top_srcdir)/.changelog.tmp; touch \$(top_srcdir)/ChangeLog; >> \ >> -echo 'git directory not found: installing possibly empty changelog.' >> >&2)" >> +echo 'git failed to create chanelog: installing possibly empty >> changelog.' >&2)" > > The filename is "ChangeLog", camel case. Not your doing, but less fix this > confusion as well. Ack. I'll check if you/others prefer the other approach (as seen in snippet) before sending with v2. >> >> AC_SUBST([CHANGELOG_CMD]) >> ]) # XORG_CHANGELOG > > > Reviewed-by: Gaetan Nadon <[email protected]> > Thanks! Emil _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
