[PATCH xinit 1/3] Drop $RAWCPPFLAGS when generating startx

[Hans de Goede]

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 <hdegoede at redhat.com>
>> ---
>>  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
> 
> 
> 
> 
> 
> 
> 
>