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
<j...@ristioja.ee <mailto:j...@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@crosswire.org
<mailto:sword-devel@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@crosswire.org
<mailto:sword-devel@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@crosswire.org
<mailto:sword-devel@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@crosswire.org
<mailto:sword-devel@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@crosswire.org
<mailto:sword-devel@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@crosswire.org
<mailto:sword-devel@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@crosswire.org
<mailto:sword-devel@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@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@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@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@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@crosswire.org
http://www.crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page

Reply via email to