Hi, On 03/25/2014 06:22 PM, Gaetan Nadon wrote: > On 14-03-25 07:56 AM, Hans de Goede wrote: >> startx.cpp contains things like #if defined(__SCO__), and >> $RAWCPPFLAGS contains -undef causing these to not get set. >> >> Signed-off-by: Hans de Goede <[email protected]> >> --- >> cpprules.in | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/cpprules.in b/cpprules.in >> index 0931bee..781676a 100644 >> --- a/cpprules.in >> +++ b/cpprules.in >> @@ -15,4 +15,4 @@ CPP_SED_MAGIC = $(SED) -e '/^\# *[0-9][0-9]* *.*$$/d' \ >> SUFFIXES = .cpp >> >> .cpp: >> - $(AM_V_GEN)$(RAWCPP) $(RAWCPPFLAGS) $(CPP_FILES_FLAGS) $< | >> $(CPP_SED_MAGIC) > $@ >> + $(AM_V_GEN)$(RAWCPP) $(CPP_FILES_FLAGS) $< | $(CPP_SED_MAGIC) > $@ > > 1. It looks like it has been this way for a very long time. Have you > investigated why it isn't a problem today?
I've not investigated, but I assume it is not a problem today since most of the #ifdef-s are for platforms which are not being used much (if at all) anymore. Note my main reasons for fixing this are: 1) If we've #ifdef's on platform specific defines in there, they should work, otherwise we should just rip them out. 2) The second patch in this patchset introduces a #ifdef __linux__ which won't work with the -undef. > 2. This is going to change the xinitrc for Apple. How confident are you > that the patch will not create a problem? > I found on the net > (http://arstechnica.com/civis/viewtopic.php?f=19&t=371721) a user > posting his copy of xinitrc for Apple. > > if [ -f "$sysresources" ]; then > > xrdb -merge "$sysresources" > > fi > > When removing -undef, my guess is that the command will be "XRDB > -nocpp -merge $sysresources". Right or wrong, I don't know. Unless apple users have custom Xresources using #ifdef's or some such this should not matter, so I believe this won't cause any issues. If we're worried about this causing issues we could start with a patch removing all the existing #ifdef blocks, since they have been dead code for a while anyways. > 3. As for SCO, this OS is no longer supported, so we will never know. Right, as said we should consider ripping out all the existing #ifdef's first. > Take a look at app/xdm/config/Xsession.cpp. > 4. Aside from "undef", there is the option "traditional" that the patch > also throws away. It has to do with the treatment of whitespace. Right, it introduces a lot of extra newlines, dropping this actually makes the generated startx script nicer to read as there are no more unnecessary large whitespace blocks > Any idea on what happens to other platforms and/or other non-gnu compilers? No, I assume that other the some newlines disappearing there as well there will be no impact, but only testing will tell for sure. > The -undef flag was added in Sept 2005. Are you running into a problem? The problem is I cannot use #ifdef for the 2nd patch in this set without fixing the -undef problem. > There is a mystery to be resolved... > http://cgit.freedesktop.org/xorg/util/macros/commit/?id=d690e4a9febd07988d149a967791c5629c17b258 > > If these defines have always been useless, a bigger cleanup could be done. Agreed on the bigger cleanup, I guess I could do that first as a preparation patch. Then effectively the only change the dropping of RAWCPPFLAGS will cause is the dropping of the traditional option. Regards, Hans > > For reference, this is how RAWCPPFLAGS get populated from util/macros: > > # Check for flag to avoid builtin definitions - assumes unix is > predefined, > # which is not the best choice for supporting other OS'es, but > covers most > # of the ones we need for now. > AC_MSG_CHECKING([if $RAWCPP requires -undef]) > AC_LANG_CONFTEST([AC_LANG_SOURCE([[Does cpp redefine unix ?]])]) > if test `${RAWCPP} < conftest.$ac_ext | grep -c 'unix'` -eq 1 ; then > AC_MSG_RESULT([no]) > else > if test `${RAWCPP} -undef < conftest.$ac_ext | grep -c 'unix'` > -eq 1 ; then > RAWCPPFLAGS=-undef > AC_MSG_RESULT([yes]) > # under Cygwin unix is still defined even with -undef > elif test `${RAWCPP} -undef -ansi < conftest.$ac_ext | grep -c > 'unix'` -eq 1 ; then > RAWCPPFLAGS="-undef -ansi" > AC_MSG_RESULT([yes, with -ansi]) > else > AC_MSG_ERROR([${RAWCPP} defines unix with or without > -undef. I don't know what to do.]) > fi > fi > rm -f conftest.$ac_ext > > AC_MSG_CHECKING([if $RAWCPP requires -traditional]) > AC_LANG_CONFTEST([AC_LANG_SOURCE([[Does cpp preserve > "whitespace"?]])]) > if test `${RAWCPP} < conftest.$ac_ext | grep -c 'preserve \"'` -eq > 1 ; then > AC_MSG_RESULT([no]) > else > if test `${RAWCPP} -traditional < conftest.$ac_ext | grep -c > 'preserve \"'` -eq 1 ; then > RAWCPPFLAGS="${RAWCPPFLAGS} -traditional" > AC_MSG_RESULT([yes]) > else > AC_MSG_ERROR([${RAWCPP} does not preserve whitespace with or > without -traditional. I don't know what to do.]) > fi > fi > rm -f conftest.$ac_ext > AC_SUBST(RAWCPPFLAGS) > ]) # XORG_PROG_RAWCPP > > > > > > > > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
