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

Jaak Ristioja jaak at ristioja.ee
Thu Jun 13 01:12:19 MST 2013 

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Troy,

It still seems that you missed one thing. Namely, the #includes with
<> still include a prefix, i.e. sound/ and omniORB4/. The Sword
#includes do not, and this by itself is a big problem which makes
header filename clashes a lot more probable, especially if other
libraries would follow suit and have the same #include convention as
Sword does.

Regarding <> and "", the latter provides a better safeguard against
including the wrong headers in some situations, especially when
dealing with multiple versions of the headers being installed at the
same time. More commonly, using "" instead of <> can also help to
simplify the build system for libraries such as Sword.

I don't consider using <> instead of "" a bug. But what I think is
very inconvenient is that one has to explicitly use
- -I/usr/include/sword/ instead of just using #include <sword/stuff.h>
in the source files.

This is cause for confusion for developers, who might think (as I did
and do):
  1) Do I need to #include <stuff.h> // Sword header ?
  2) If I #include <sword/stuff.h> then why do I need to pass an extra
- -I argument to the compiler?

What I'm asking is that even if you don't want "" instead of <>,
please use a sword/ prefix for all respective #includes in all header
files of Sword.

Blessings,
Jaak


On 13.06.2013 09:47, Troy A. Griffitts wrote:
> 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
> 
> 
> 
> _______________________________________________ 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
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.20 (GNU/Linux)

iQgcBAEBAgAGBQJRuX7eAAoJEEqsYmEt1rCOU0c//j/5yHbZaDkg334zo8FInJ4a
1JWSaucfqIx+GhWstlrDuvMjB6+Pf4J7JQrep5csq9YI0/aBwW7yMoXRkJJjcQ8e
m3AbAGCU8qkDmTETsF/YQ7IJaiDW3kZCTaiX2LqYkWBJoubdtRaUADrL0NcSZUDv
b1uCRDABBxe6Wn89nOLq4M0DP/QFcqFjVYnt9GYtbzxEIZnBZTYXyhWPgEwHTx+f
PIubw3XfqL+feVjSIOkRcX1s5gO7FgR8CjX6Z8CNiKNmZSJmtTD/VOrh7W7YCV9E
1q/Kwfk3T9pnAm9mXx/A8sGX1ahIZdM9wRspAubvP+IorRWfzk3bC8khn3bk1Ivj
PCv0f59woJbZ14TPZUb2jCCxHwBbPDrj0iBVBcHVVVD75K12sUrPf+lRGNq3wJy6
hnoDO+10zJAezOqyFaCgPpk4Q8Mkb+bhoSAF4GiXc30j+aqOwU7U7FKTGXyqVRQ+
5vp2mGlhL3VFbuFZjnTguQaxQewkSAJu1lw5bTjWeVismo5IcYXbokN8Fn3nPOAK
b1n1oQvbsTWIMhVTrLxuYMDU5zA1V4GrtqDVZ4k2ZXhbtRsGSX1MQuZgRZXIzpaR
AKnWJVwKtN2rX8b+iBCeLkG++qjHq0McD3f2JdkaF4v0YtwHpqfuWTyzPzztriwG
7HLE/02y5jv8Cv/5YKXjO87FX6EdbZIRDo1g7BeszIKdnvvOQYNPCfIDWXjaS6ge
D38wdRJu6yt3jGt2ox5AOGFpkGS7rqKK6D2RF9N2cbcLAjrL/aFY/pireh1s26Kf
iWO9QWy+8eNyzW+PBdXxxwjWVrgDKjhNNKdBAMUauX4KRmWLNQPbXteSJ6tnC6/s
b2sxC32K4oFTDdlfwcu67O1fqwOvpO6J5dXzVBbSrZFJbqf3WpQEQT1w5kGrEt/9
Y+xRz7HLHr9xvHVtbuJqzEnXQsicdBvUFggaIpnfUp0oE06q+UNF78EczkbwuFCL
JFebGJNF8vf7Ek9ZhRJALghgC9RJo5zZOzpy9+46TjNR5D4na80LRn3xYF0y1/8n
Z0WC1YcxVt03iWCxk11YRSqHBXyELWh7bG9w3PxDUXj/OBA2LSrMcuwLk0la1cBW
0RX8rVku4GqJucGjavUUCAWy2RxACK4bjzGJ9c0LA42/GR4V2QCnXdWWNo0sX4g1
1sK8JF4NjUPcXp9SpDNKmOSCP/rK6zjOQbdY2hn5kmsOvPfpFYkB9G9Zn4CzLOsv
zJsIOyXbgJ29+XlI0Nh/4JU95zs6bAoEdjuJctnCqRl4uU+mjcAcTs99exfvQ10M
i0NsekXFl+7CFDxMeUMsdR/+pdTgacQTkTkl/Bo2b2M7/TfkXfn3PX/YD9t2vYS+
uFirQSs76OZAGtMUhlIGfxUbxJWZakSKp9VpRfkZ/5Koz251tYM3U7xGD9Kjf5E6
aKG1sf/1OQwz5x+gUzrKOKaDzoW45df3+rGlglLGPurDa16MTsHWjNYwYHnKLgyL
raXpQZF+tRze1BirFQ9zhFCU4wu/krmq4Oka+FzBVMDITQ1JWy8oWZLDtEFSgiYh
Y/+gss7RpFXRh0MbFMK43Bpo/x4Ug3cw+j9oeQdfBoj21WRZNuyLYJ0GIUk+u3FO
fNg/hjqNt9WOxlEWgIqWQ4Vg4C2dOFf92FYyCy/LHHz2jUpx4pNwjf3ET7D3VJou
L4S3S0qz31n6gQBxD9gcfL3JFib1BPhsXwNwlAxYlKUsml/FzTK9ChKWKynVnJmn
/PfR1xyejpE2+3NXz3NtCR7NO8EqIkd6tSS9qPgirENc7usIVgwttIcg4kowz4qN
Dy6Gl7YPG+NH3hcpJNhMsCxuPxPWwYTYlLPNgHVP7ES4HoHuYXlZ6X1aa4zSU+rU
MLb9WiJWK3luIDpXJ3DWro45kepYF+LhYdUO8Mj5halXDZZx7oe9rNzN+MNJmq3C
zRYfH6BdyBtyZjRBK5L8xgOeknw9vJMtsI8mlxEksVENmXxPDMF38cBrkRiLC3Nz
00GTHAcHIHPiF/ND3VOE70FKuU7bg2re/QHS/O8LcWoDDrMiDMjwysDIpzjLZtxX
vWGgGKqRljHQGjdDbZxiElI+gaLBrSfq8QM3+Y8KoUzEGIqBwyF9Cg6GvolNXqWD
XtsIg8G/LsYgXp+8rVNynkHRplXZBHv2KOBU4vbU10sEkFrcaQaes3In1oPQWpFj
wZo/Avd7inY8vDm9iqWzn0pYCQmIXiX0sfyg/MVyU09aZEfmsxvorN0gPta+0ltv
FRaQwkk6nbDBxH5NaLT7C+CFw3ngNjcg+Sg/UkSMEYonydYMMytpQUpF8U0qHS7T
I5llUkesRvd4kuc2Y/XrnoLRiETX1EhICS8tDwREnpbNeQpQkjh7TFI44yhzj5M0
ugOXUWS7m9MZNmPOLVZ5cIlblm9vOP7+PqgDeESNJUeVk7nclJjHVI/hm4Ne0LtU
7N8INF8hcso+j2XYe4RBseHaEAUDTL6q6s5XCowKQBo6zRw+hScMsvv/g9RgqOER
7gELk2ideppWt7zsKno2rhHsYgObw9cANGzI3wTyEdi6ar8SaWqK9NGUcdyekX8r
i2fKxr8L5xfYAlri7AFQDKc6TzElnHusm2hmL+iU7ORuBN/WE1fhdZ0YWzQ+KmIQ
DsrorR1Wj5SyenSySTJa/MRugige2x2DKwI6LUlD+294W0dO2ApyiBRYahIurexG
FE64cElvPOHVez1SOKTp
=cZvX
-----END PGP SIGNATURE-----

More information about the sword-devel mailing list