sessreg: Changes to 'master'

[Dan Nicholson]

On Tue, Mar 15, 2011 at 5:32 AM, Jon TURNEY <jon.turney at dronecode.org.uk> wrote:
> On 14/03/2011 17:53, Gaetan Nadon wrote:
>> On Mon, 2011-03-14 at 08:18 -0700, Dan Nicholson wrote:
>>
>>> On Mon, Mar 14, 2011 at 7:31 AM, Jon TURNEY <jon.turney-GrJqePx9RPPAJUdA+FbntA at public.gmane.org> wrote:
>>>> On 19/01/2011 15:49, Gaetan Nadon wrote:
>>>>>  Makefile.am         |   25 ---------
>>>>>  configure.ac        |    5 +
>>>>>  filenames.sed.c     |   29 -----------
>>>>>  man/Makefile.am     |   22 ++++++++
>>>>>  man/filenames.sed.c |   29 +++++++++++
>>>>>  man/sessreg.man     |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>  sessreg.man         |  135 ----------------------------------------------------
>>>>>  7 files changed, 192 insertions(+), 188 deletions(-)
>>>>>
>>>>> New commits:
>>>>> commit 3bb9256b8e3a56b94281ea787c07f080c0a80312
>>>>> Author: Gaetan Nadon <memsize-XzQKRVe1yT0V+D8aMU/kSg at public.gmane.org>
>>>>> Date:   Wed Jan 19 10:06:55 2011 -0500
>>>>>
>>>>>     config: move man pages into their own directory
>>>>>
>>>>>     Use services provided by XORG_MANPAGE_SECTIONS.
>>>>>     Use standard Makefile for man pages.
>>>>
>>>> I can't tell for sure since sessreg has only recently been added to the
>>>> xorg.modules list and thus gets built by tinderbox, but it looks like this
>>>> change broke builddir != srcdir builds, see [1]. (there's also a separate
>>>> cygwin platform issue with WTMPX_FILE I need to look)
>>>>
>>>> Patch attached for your consideration.
>>>>
>>>> [1] http://tinderbox.freedesktop.org/builds/2011-03-13-0008/logs/sessreg/#build
>>>
>>> Oh, I think I know the reason. -I$(top_srcdir) would normally be part
>>> of $(INCLUDES). However, since the man page moved to its own
>>> subdirectory, this variable (and others related to C code compilation)
>>> are not set by automake. This is because there are no actual C code
>>> rules in that Makefile.am. When it was in the top directory, it got
>>> these because sessreg.c was getting built there.
>>>
>>> I think to be correct you need -I$(top_srcdir) and -I$(top_builddir)
>>> so you get both sessreg.h and config.h (included through sessreg.h).
>>> The alternative is to leave filenames.sed.c in the top directory and
>>> ensure it gets built before descending to the man directory.
>>>
>>
>>
>> I checked-out a new sessreg clone and I cannot reproduce the problem.
>>
>> This is where master is pointing:
>> http://cgit.freedesktop.org/xorg/app/sessreg/commit/?id=5a886e2fc5f4b3f062a20f66ee68bad4a0b98ad1
>>
>> I ran make distcheck both from srcdir and from a VPATH build dir.
>>
>> Can you reproduce outside tinderbox?
>
> Yes, always reproduces for non-srcdir builds for me.
>
> $ autoconf --version
> autoconf (GNU Autoconf) 2.68
> $ automake --version
> automake (GNU automake) 1.11.1
> $ git clone git://anongit.freedesktop.org/xorg/app/sessreg
> [...]
> $ cd sessreg
> $ git describe
> sessreg-1.0.6-8-g5a886e2
> $ mkdir build ; cd build
> $ ../autogen.sh
> [...]
> $ make V=1
> make  all-recursive
> make[1]: Entering directory `/opt/wip/sessreg/build'
> Making all in man
> make[2]: Entering directory `/opt/wip/sessreg/build/man'
> gcc -E -DHAVE_CONFIG_H   \
>            -I..  ../../man/filenames.sed.c | \
>            grep 's|__' > filenames.sed
> ../../man/filenames.sed.c:1:21: error: sessreg.h: No such file or directory
>
>
> Another wrinkle is that the use of 'cpp foo | grep' in a Makefile doesn't seem
> like a good idea, as it can silently fail (see [1]) (man bash tells me this is
> because the "status of a pipeline is the exit status of the last command")
>
> [1] http://tinderbox.freedesktop.org/builds/2011-03-14-0008/logs/sessreg/#build

Well, this is certainly not the only module where we pipe the output
of cpp to grep or sed. You could use an extra temporary file instead
of a pipe if you're paranoid.

--
Dan