From: Keith M. <kei...@us...> - 2011-12-24 22:30:33
|
I'd appreciate some further cygwin specific comment regarding the following analysis:-- On 07/12/11 14:08, Keith Marshall wrote: > I am following up on the issues raised in: > https://sourceforge.net/tracker/?func=detail&atid=102435&aid=3063296&group_id=2435 > https://sourceforge.net/tracker/?func=detail&aid=3447137&group_id=2435&atid=302435 > https://sourceforge.net/tracker/?func=detail&atid=102435&aid=3441135&group_id=2435 If we consider the w32api package, (so ${top_srcdir} would refer to a working copy of the src/winsup/w32api directory from sourceware.org), and we examine Makefile.in respectively in each of ${top_srcdir}/lib, ${top_srcdir}/lib/ddk and ${top_srcdir}/lib/directx, we see several references of the form ${srcdir}/.. and ${srcdir}/../..; these have clearly been manually encoded by the original maintainer, with intent to refer to ${top_srcdir}. This method of encoding is inherently unreliable, particularly since autoconf provides a definition for ${top_srcdir} which is guaranteed to generate the correct reference. Given that, in each case, ${srcdir} will always refer to directory in which the Makefile.in itself resides, in ${top_srcdir}/lib/Makefile.in any reference to ${srcdir}/.. must refer to ${top_srcdir} itself. By the same reasoning, any reference in ${top_srcdir}/lib/ddk/Makefile.in or in ${top_srcdir}/lib/directx/Makefile.in to ${srcdir}/../.. is also a reference to ${top_srcdir}. Now if I normalise all such ${srcdir}/.. and ${srcdir}/../.. to make them directly refer to ${top_srcdir}, (and if I elide references which are not germane to the point I wish to discuss), in lib/Makefile.in I see the following:-- $ grep '${top_srcdir}' lib/Makefile.in EXTRA_INCLUDES = -I ${top_srcdir}/../include \ -I ${top_srcdir}/../../newlib/libc/include \ -I ${top_srcdir}/../../newlib/libc/sys/cygwin EXTRA_INCLUDES = -I ${top_srcdir}/../mingw/include INCLUDES = -I ${top_srcdir}/include $(EXTRA_INCLUDES) (the first EXTRA_INCLUDES definition is cygwin specific; the second relates to MinGW). This seems mostly correct, with two exceptions:-- 1) ${top_srcdir}/../mingw/include is correct for a full winsup check out, but needs a fix-up for a tree built manually from MinGW.org source packages; I propose handling that using a --with-mingwrt-srcdir option to configure, similar to the --with-w32api-srcdir option proposed for the mingwrt package,on the third of the above tickets; it could use _mingw.h as a representative file for directory identification. 2) ${top_srcdir}/../include (in the cygwin EXTRA_INCLUDES) looks bogus; it would refer to src/winsup/include on sourceware.org, (and a comment in the file suggests that this is the intent); however, there is no such directory in the repository. Is this an error? Should I simply discard this phantom reference? Or, given this cygwin specific comment relating to the definition of EXTRA_INCLUDES: ifeq ($(BUILDENV), cygwin) # winsup/include # winsup/../newlib/libc/include # winsup/../newlib/libc/sys/cygwin do the cygwin developers anticipate that the currently non-existent src/winsup/include directory will become an imminent requirement? Or was the intent to refer to (libiberty's?) src/include directory, (from one level higher in the repository tree)? If so, is this needed? In lib/ddk/Makefile.in, and in lib/directx/Makefile.in, I see similar definitions:-- $ grep -r top_srcdir lib/ddk/Makefile.in EXTRA_INCLUDES = -I ${top_srcdir}/include \ -I ${top_srcdir}/../newlib/libc/include \ -I ${top_srcdir}/../newlib/libc/sys/cygwin EXTRA_INCLUDES = -I ${top_srcdir}/mingw/include $ grep -r top_srcdir lib/directx/Makefile.in EXTRA_INCLUDES = -I ${top_srcdir}/include \ -I ${top_srcdir}/../newlib/libc/include \ -I ${top_srcdir}/../newlib/libc/sys/cygwin EXTRA_INCLUDES = -I ${top_srcdir}/mingw/include However, these are clearly incorrect -- it appears that the original definitions, in terms of ${srcdir}/../, were simply copied verbatim from lib/Makefile.in, without regard for the extra level of removal from ${top_srcdir}. The same is true of the definition of INCLUDES, which also appears in each of these two files, but, lacking the appropriate ${srcdir}/../../ reference, escaped substitution: s,${srcdir}/\.\./\.\./,${top_srcdir}/,g so it remained (incorrectly specified), in lib/ddk/Makefile.in, as: INCLUDES = -I ${srcdir}/../include $(EXTRA_INCLUDES) and in lib/directx/Makefile.in, as: INCLUDES = -I ${srcdir}/../include \ -I ${srcdir}/../include/directx \ $(EXTRA_INCLUDES) Indeed, this is the underlying issue which each of the first two tickets sets out to address; the foregoing analysis shows that neither of the two patches originally proposed adequately addresses it. Thus, I propose to:-- 1) Replace all unreliable references of the form ${srcdir}/../... with normalised and corrected references, relative to ${top_srcdir}, and 2) Factor out the common EXTRA_INCLUDES definitions, into a single Makefile.comm.in file, to be included by all subdirectory makefiles as required, (as I've already done for other common makefile code). However, I would appreciate some guidance from the cygwin experts, on the following cygwin specifics:-- 1) Do we keep the reference to the non-existent winsup/include? If so, do we need a --with-winsup-include option to locate it? If we do, what possible criterion can we adopt to auto-detect it? 2) Should we provide a --with-libc-srcdir option to specify the location of the newlib headers, (as we need --with-mingwrt-srcdir)? If so, is existence of newlib.h a suitable auto-detection criterion? Is there a more suitable alternative? 3) The newlib/libc/sys/cygwin directory is currently empty; it did have content at one time, but this has been removed. Should we now drop the reference from w32api's makefiles? -- Regards, Keith. |