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

Jaak Ristioja jaak at ristioja.ee
Wed Jun 12 13:28:59 MST 2013 

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
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 2916 bytes
Desc: OpenPGP digital signature
URL: <http://www.crosswire.org/pipermail/sword-devel/attachments/20130612/1fab48a2/attachment-0001.sig>

More information about the sword-devel mailing list