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

Thu Jun 13 01:32:02 MST 2013

Jaak,

On 06/13/2013 10:12 AM, Jaak Ristioja wrote:
> -----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.
Jaak,  I didn't miss this.

As I said in my last email, I'm not saying your wrong.  I'm just saying:

a) the headers I looked at include things DIFFERENTLY than your patch, 
so I'm not sure your fix is the most standard.
b) things work right now on a ton of platforms and compilers and build 
projects, and I don't want to change this just before a release, which 
could potential break all of these-- especially if there are no known 
problems with this right now.  There is no reason to push a potentially 
disruptive patch just before a release.

Also, if you use pkg-config to get the compile and link directives, you 
shouldn't need to care about what to pass to the compiler.

.cpp:
      g++ `pkg-config --cflags sword` $< -o $@ `pkg-config --libs sword`

Thanks for being concerned.  I understand your concern and value your input,
Troy

>
> 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-----
>
> _______________________________________________
> 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