[PATCH util/macros] Rework stderr messages for the _CMD macros

[Emil Velikov]

On 31 January 2017 at 21:05, Gaetan Nadon <memsize at videotron.ca> wrote:
> On 01/31/2017 05:26 AM, Emil Velikov wrote:
>>
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> 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 <memsize at videotron.ca>
>> Cc: Peter Hutterer <peter.hutterer at who-t.net>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>> 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 <memsize at videotron.ca>
>
Thanks!

Emil