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

[Hans de Goede]

Hi,

On 03/26/2014 02:21 PM, Gaetan Nadon wrote:
> On 14-03-26 07:37 AM, Hans de Goede wrote:
>> 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.
> Yes.
>>
>> 2) The second patch in this patchset introduces a #ifdef __linux__ which won't work with
>> the -undef.
> I see the problem.
>>
>>>  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.
> That would be prudent. Those who have specific platform skills could review.
> It also leaves a better trace in git as to what and why things have been
> done.
>>
>>>  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.
> Ok.
>>
>>> 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.
> Ok, it makes sense. Let's fix the cpp source first. The content of
> RAWCPPFLAGS vary by platform and compiler, so it is difficult to review.

So I've been working on this today, and startx.cpp is full of #ifdef -s not only
for sco but also for apple, linux, etc. Most of these are handling of platform
specific config-files. So we likely have not been getting any bugreports since
the defaults happen to work for most users just fine too (and apple X users likely
end up reporting the bug elsewhere anyways).

Still it seems wrong to me to just rip out all this special handling. I believe
most of it is still valid. So I'm going to do a new version of this patchset,
removing the SCO stuff, but leaving the rest in.

I'm also going to just drop -undef and keep -traditional to make sure that the
dropping of -traditional does not cause any undesirable side-effects.

Regards,

Hans