[sword-devel] Changing #include structure (was: I implore you...)

Troy A. Griffitts scribe at crosswire.org
Wed Jun 12 23:47:36 MST 2013 

Jaak,

I'm of the same mind as Greg.  Our include syntax has been working as is 
for 20 years on a number of compilers and platforms.  I hesitate to 
wholesale change this now because of 'in principle' arguments without 
any actual problems being seen.  I don't have time to compile your 
changes on all the platforms we support to test them.  I know what we 
have now works.

I'm not saying that you're not right, but I also don't see your changes 
as standard.  Just a brief look on my box, 
/usr/include/sound/sfnt_info.h includes /usr/include/sound/asound.h with:
#include <sound/asound.h>

/usr/include/omniORB4/anyStream.h includes 
/usr/include/omniORB4/omniTypedefs.hh with:
#include <omniORB4/omniTypedefs.hh>

Just the first 2 I looked at.

I'd like more time to investigate this before making a change,

Troy




On 06/12/2013 10:28 PM, Jaak Ristioja wrote:
> Well, using -I/usr/include/sword is just an ugly workaround. It doesn't
> change the fact that Sword headers include each other in a wrong manner
> (using <> instead of ""). Because it would only be a workaround, we do
> not want to stick to it forever. We want this fixed.
>
> Secondly, headers in the Sword include directory have a greater chance
> of colluding with other headers in /usr/include/ and elsewhere. Hence
> the Sword headers themselves might include wrong header files if that
> happens. That's why using "" instead of <> is important so that relative
> paths would be searched before any locations specified by -I arguments
> to the compiler.
>
> I don't believe my patch broke anything, the main changes were
> substituting <> with "" when #including Sword headers. However, if it
> did break anything, it should be easy to amend. I'm highly motivated to
> get my patches applied to the next version of Sword one way or the
> other. So just ask and I'll fix it.
>
> Blessings,
> Jaak
>
>
> On 12.06.2013 23:05, Greg Hellings wrote:
>>
>>
>> On Wed, Jun 12, 2013 at 2:51 PM, Jaak Ristioja <jaak at ristioja.ee
>> <mailto:jaak at ristioja.ee>> wrote:
>>
>> Hi!
>>
>> What's the integration status on this patch?
>>
>> Blessings,
>> Jaak
>>
>>
>>> Can we allow changes which are not bare-minimum build-breakers, such as
>>> restructuring the includes, be a later issue for the next next release
>>> and just get 1.7.0 out the door, please?
>>> Also, what prevents you from having
>>> -I/usr/include -I/usr/include/sowrd
>>> and then having #include <sword/foo.h> and having it all work just as
>>> planned?
>>> --Greg
>>
>>
>> PS: Is Troy the only one with access to apply this?
>>
>> On 10.06.2013 22:49, Jaak Ristioja wrote:
>>> Argh, someone must have changed things on SVN lately, so this
>>> patch was invalid for the current trunk... I wish you guys would
>>> learn git or something. Anyway, here's something which should apply
>>> to SVN 2819, I hope.
>>> SHA1SUM: 071a4fb64f1d0c2ed5d746d08791592f76eaf633
>>> Blessings, Jaak
>>> On 10.06.2013 22:34, Jaak Ristioja wrote:
>>>> Attached is a patch for this. Please apply.
>>>> SHA1SUM: 9a99e34ce419ea3288a32148d431ec971fb0e675
>>>> Blessings, Jaak
>>
>>>> On 10.06.2013 19:38, Jaak Ristioja wrote:
>>>>> I'm working on the patch but here's a short overview of the
>>>>> problem, in case discussion is required. The problem is that
>>>>> source code using Sword can't do stuff like:
>>>>> #include <sword/versekey.h>
>>>>> This is VERY BAD, because we must do
>>>>> #include <versekey.h>
>>>>> and provide -I/path/to/sword/includes/ to the compiler every
>>>>> time. The problem with this approach is that versekey.h might
>>>>> also exist in /usr/include or in other -I/include/paths.
>>>>> Additionally, this makes the #include list rather
>>>>> incomprehensible, especially when we want to sort it
>>>>> alphabetically. There's no telling what <versekey.h> refers to
>>>>> - is it part of Sword, part of something else, or a typo (e.g.
>>>>> maybe this needs to be "versekey.h").
>>>>> Why #includes like <sword/versekey.h> don't work is that the
>>>>> Sword headers themselves use includes like <versekey.h>
>>>>> instead of "versekey.h" which is correct. If I don't include
>>>>> -I/usr/include/sword in my compiler arguments, but #include
>>>>> <sword/versekey.h>, the versekey.h file tries to #include
>>>>> <swkey.h> which fails because it can't find the file in in the
>>>>> include path.
>>>>> The *.cpp files in Sword also need to use "" instead of <> to
>>>>> distinguish between header system and local header files.
>>>>> Afaik this is just best practice. Existing code using #include
>>>>>   <versekey.h> etc will continue to work as long as the
>>>>> -I/path/to/sword/includes exists.
>>
>>>>> Blessings, Jaak
>>>>> On 10.06.2013 19:21, Jaak Ristioja wrote:
>>>>>> Actually I just remembered another serious flaw which causes
>>>>>> a headache for developers using Sword. I'll write a patch
>>>>>> ASAP.
>>>>>> Blessings, Jaak
>>>>>> On 10.06.2013 09:43, Troy A. Griffitts wrote:
>>>>>>> Jaak,
>>>>>>>
>>>>>>> I accepted and applied your header file patch nearly 5
>>>>>>> months ago.  Are you telling me that you still have 549
>>>>>>> warnings from SWORD headers?
>>>>>>>
>>>>>>> Troy
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 06/09/2013 11:55 PM, Jaak Ristioja wrote: On 09.06.2013
>>>>>>>   23:21, Troy A. Griffitts wrote:
>>>>>>>>>> I don't think other developers are getting ignored.
>>>>>>>>>> Please be specific.  Just because I don't accept a
>>>>>>>>>> patch doesn't mean a developer is getting ignored.
>>>>>>>>>>
>>>>>>>>>> In fact, many times trying to make this release, when
>>>>>>>>>>   people complain that we need something fixed for
>>>>>>>>>> this release, I ask for a simple testsuite addition
>>>>>>>>>> to show the problem and desired result, and don't get
>>>>>>>>>> a response.
>>>>>>>>>>
>>>>>>>>>> I don't believe the problem is as you think it is
>>>>>>>>>> Jaak. Many people whine about this or that. Not all
>>>>>>>>>> whine for things to go in the same direction.
>>>>>>>>>>
>>>>>>>>>> Everyone whines for a release but not everyone is
>>>>>>>>>> willing to help submit tests and then fixes for those
>>>>>>>>>>   tests.
>>>>>>>>>>
>>>>>>>>>> You stated that you would get involved to help, but
>>>>>>>>>> you only submit things for which I previously told
>>>>>>>>>> you I wasn't interested in accepting (worrying about
>>>>>>>>>> pedantic warnings whose changes often make the code
>>>>>>>>>> less readable and do nothing to improve any of the
>>>>>>>>>> real problems for the end user.  Though I do
>>>>>>>>>> appreciate a few of the warning fixes you submitted,
>>>>>>>>>> a few being actual bug fixed too (thank you)-- I'm
>>>>>>>>>> just ranting right now.)
>>>>>>> As a BibleTime developer, I want to available tools (-Wall,
>>>>>>>   -Wextra, cppcheck, etc) to fix any errors in my code. Due
>>>>>>> to the Sword header files which generate a lot of warnings
>>>>>>> this task is VERY inconvenient. For example, when I compile
>>>>>>> the whole of BibleTime with GCC, I get 549 warnings from
>>>>>>> Sword headers (mostly for unused arguments) - how am I
>>>>>>> supposed to find the warnings relevant for BibleTime? This
>>>>>>> alone often makes it a pain to develop BibleTime and gives
>>>>>>> me enough reason to want to fork Sword.
>>>>>>>
>>>>>>> Turning on and fixing pedantic warnings will help find real
>>>>>>>   bugs. FACT! Forcing developers to work blindfolded will
>>>>>>> not help anyone.
>>>>>>>
>>>>>>> The same tools can be used to find bugs in Sword code, and
>>>>>>>   SHOULD regularly be used for this purpose to ensure code
>>>>>>> quality. As is obvious these are currently NOT BEING USED
>>>>>>> by Sword developers. However, when things eventually
>>>>>>> break, users complain to the BibleTime project. Hence, it
>>>>>>> is also in the interests of front-ends to ensure that the
>>>>>>> code of Sword is of good quality. Again - if Sword won't
>>>>>>> work to ensure this and wont let us in to fix things, we
>>>>>>> have another reason to fork.
>>>>>>>
>>>>>>> This again leads us to the issue of attracting new
>>>>>>> developers to Sword. I don't want to write on this more
>>>>>>> than necessary to provide a small argument for my
>>>>>>> conclusion. Afaik the current situation isn't working well.
>>>>>>> Biggest obstacles for me personally include working
>>>>>>> blindfolded, submitting patches by e-mail and not getting
>>>>>>> enough feedback for (ignored) patches and other emails.
>>>>>>>
>>>>>>> To conclude - maybe its just me, but altogether I really
>>>>>>> feel it were easier to maintain a parallel fork (at
>>>>>>> minimal to provide set of patches) than to waste my time
>>>>>>> writing long letters trying to make this relationship work
>>>>>>> in its current form. I accept whatever path the Sword
>>>>>>> project takes, but if it's not enough for the needs of
>>>>>>> BibleTime and our devs, we will make our own choices as
>>>>>>> well.
>>>>>>>
>>>>>>>
>>>>>>> Blessings, Jaak The BibleTime team
>>>>>>>
>>>>>>>
>>>>>>> PS: I apologize if this late-night response is
>>>>>>> incomprehensible.
>>>>>>>> _______________________________________________
>>>>>>>> sword-devel mailing list: sword-devel at crosswire.org
>> <mailto:sword-devel at crosswire.org>
>>>>>>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>>>>>>> Instructions to unsubscribe/change your settings at above
>>>>>>>>   page
>>>>>>>
>>>>>>> _______________________________________________ sword-devel
>>>>>>>   mailing list: sword-devel at crosswire.org
>> <mailto:sword-devel at crosswire.org>
>>>>>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>>>>>> Instructions to unsubscribe/change your settings at above
>>>>>>> page
>>
>>
>>
>>>>>> _______________________________________________ sword-devel
>>>>>> mailing list: sword-devel at crosswire.org
>> <mailto:sword-devel at crosswire.org>
>>>>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>>>>> Instructions to unsubscribe/change your settings at above
>>>>>> page
>>
>>
>>>>> _______________________________________________ sword-devel
>>>>> mailing list: sword-devel at crosswire.org
>> <mailto:sword-devel at crosswire.org>
>>>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>>>> Instructions to unsubscribe/change your settings at above page
>>
>>
>>
>>
>>>> _______________________________________________ sword-devel
>>>> mailing list: sword-devel at crosswire.org
>> <mailto:sword-devel at crosswire.org>
>>>> http://www.crosswire.org/mailman/listinfo/sword-devel
>>>> Instructions to unsubscribe/change your settings at above page
>>
>>
>>
>>
>>> _______________________________________________ sword-devel mailing
>>> list: sword-devel at crosswire.org <mailto:sword-devel at crosswire.org>
>>> http://www.crosswire.org/mailman/listinfo/sword-devel Instructions
>>> to unsubscribe/change your settings at above page
>>
>>
>>      _______________________________________________
>>      sword-devel mailing list: sword-devel at crosswire.org
>>      <mailto:sword-devel at crosswire.org>
>>      http://www.crosswire.org/mailman/listinfo/sword-devel
>>      Instructions to unsubscribe/change your settings at above page
>>
>>
>>
>>
>> _______________________________________________
>> sword-devel mailing list: sword-devel at crosswire.org
>> http://www.crosswire.org/mailman/listinfo/sword-devel
>> Instructions to unsubscribe/change your settings at above page
>>
>
>
>
> _______________________________________________
> sword-devel mailing list: sword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.crosswire.org/pipermail/sword-devel/attachments/20130613/773ac801/attachment-0001.html>

More information about the sword-devel mailing list