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 <[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. > 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
