Re: [sword-devel] Cross references with Sword 1.9.0RC3
Jaak, I did not get an error about this. The capitalized version of this function does exist in sword, so override would not help. It looks like it is implemented reasonably, but it did not work for me. I am quite confident that I was not mixing versions of sword. Gary On 10/18/20 1:07 PM, Jaak Ristioja wrote: Gary, are you sure that the compiler did not error or at least warn you about this? Because strangely enough, in BibleTime, the overridden CSwordBackend::AddRenderFilter method already seems to have the override keyword present. I suspect what you saw might instead have been caused by something like compiling BibleTime against an older version of Sword, but running it against the newer version of the Sword dynamic library. This is a form of API breakage which might perhaps lead to results like this. If this is the case we might in the future get bug reports about this from downstream as well. J On 18.10.20 16:40, Jaak Ristioja wrote: Hi! The respective change in Sword is SVN 3753: https://github.com/bibletime/crosswire-sword-mirror/commit/b684b6f099 It seems that this time we were not lucky to have a SWDEPRECATED alias for the old function name, silently breaking API, and hence BibleTime. In theory this silent error could also have been detected in BibleTime if we would have used the override keyword. In practice, finding all the functions which need the override keyword is for most projects only easy to do if the -Wsuggest-override flag (and perhaps even -Werror=suggest-override for debug builds) is used. Unfortunately for BibleTime (and other projects using Sword), just turning this flag on yields very many warnings from Sword headers themselves. The task of figuring out how (and when in the build process) to filter BibleTime-specific warnings from all warnings is likely not a trivial one. Best regards, J On 18.10.20 01:50, Gary Holmlund wrote: I found the issue. BibleTime is using a sword function, AddRenderFilters, which is deprecated. It was renamed to addRenderFilters (A -> a). Gary On 10/17/20 1:36 PM, Gary Holmlund wrote: I have found that sword is not calling the BibleTime OsisToHtml filter. It is called with 1.8.1 and not with 1.9RC3. I am still working on why this does not happen. Gary On 10/16/20 3:49 AM, Troy A. Griffitts wrote: Hi Gary. No, nothing should have changed in that respect. Can anyone else confirm they are having trouble with these in our last RC or svn HEAD? On October 16, 2020 2:44:32 AM GMT+02:00, Gary Holmlund wrote: >I switched to sword 1.9 rc3 in BibleTime. I am on Fedora 33. Now I have > >lost cross references, footnotes, Jesus words, etc. > >Did any api for references change? Any other thoughts? > >Gary Holmlund > > >___ >sword-devel mailing list: sword-devel@crosswire.org >http://crosswire.org/mailman/listinfo/sword-devel >Instructions to unsubscribe/change your settings at above page -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page
Re: [sword-devel] SVN 3813 makes compilation of Sword fail
"Added extra cast (int)(size_t) to avoid stupid clang error that doesn't
like void * being cast (int) directly to an int."
UH-OH!!!
First of all, the build that failed for BibleTime was using GCC not Clang.
Secondly, the compiler is correct to warn, because a pointer does not
always fit into an int, e.g. for the LLP64 an LP64 data models [1]. So
it seems that you might be throwing away half of the bits and expect it
to always work. For those data models it might work if your OS only
gives you addresses in the range of [0, 2^32) but that is not always
guaranteed, leading to undefined behavior.
In short, please don't store pointers in integers other than intptr_t
and uintptr_t and convert them using reinterpret_cast! See also:
http://eel.is/c++draft/expr.reinterpret.cast#5
http://eel.is/c++draft/basic.stc.dynamic.safety
Best regards,
J
[1] https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models
On 19.10.20 01:23, Troy A. Griffitts wrote:
Crapola. Should be fixed. This was intended to be a call to the new
FileMgr::write which hides the OS-specific impl, but I was configured to
use CURL instead of FTPLib, so I missed the compilation error. The
Android port uses FTPLib and I just built there successfully with the
committed I just pushed. Thank you Jaak.
Troy
On 10/18/20 11:57 PM, Jaak Ristioja wrote:
Hello!
The commit "A bit more work on making it easier to use SWORD in a
threadsafe manner." makes compilation of Sword fail:
src/mgr/ftplibftpt.cpp: In function ‘int
sword::{anonymous}::my_filewriter(netbuf*, void*, size_t, void*)’:
src/mgr/ftplibftpt.cpp:52:21: error: cast from ‘void*’ to ‘int’ loses
precision [-fpermissive]
int output = (int)fd;
^~
src/mgr/ftplibftpt.cpp: In function ‘int
sword::{anonymous}::my_filewriter(netbuf*, void*, size_t, void*)’:
src/mgr/ftplibftpt.cpp:52:21: error: cast from ‘void*’ to ‘int’ loses
precision [-fpermissive]
int output = (int)fd;
^~
src/mgr/ftplibftpt.cpp:53:3: error: ‘write’ was not declared in this
scope
write(output, buffer, size);
^
src/mgr/ftplibftpt.cpp:53:3: error: ‘write’ was not declared in this
scope
write(output, buffer, size);
^
src/mgr/ftplibftpt.cpp:53:3: note: suggested alternative: ‘fwrite’
write(output, buffer, size);
See https://github.com/bibletime/bibletime/runs/1272250545 for the
failing build run.
Commit details in our git mirror of the Sword SVN repository:
https://github.com/bibletime/crosswire-sword-mirror/commit/c52559ecae
Best regards,
J
___
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
___
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
___
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
Re: [sword-devel] SVN 3813 makes compilation of Sword fail
Crapola. Should be fixed. This was intended to be a call to the new
FileMgr::write which hides the OS-specific impl, but I was configured to
use CURL instead of FTPLib, so I missed the compilation error. The
Android port uses FTPLib and I just built there successfully with the
committed I just pushed. Thank you Jaak.
Troy
On 10/18/20 11:57 PM, Jaak Ristioja wrote:
Hello!
The commit "A bit more work on making it easier to use SWORD in a
threadsafe manner." makes compilation of Sword fail:
src/mgr/ftplibftpt.cpp: In function ‘int
sword::{anonymous}::my_filewriter(netbuf*, void*, size_t, void*)’:
src/mgr/ftplibftpt.cpp:52:21: error: cast from ‘void*’ to ‘int’ loses
precision [-fpermissive]
int output = (int)fd;
^~
src/mgr/ftplibftpt.cpp: In function ‘int
sword::{anonymous}::my_filewriter(netbuf*, void*, size_t, void*)’:
src/mgr/ftplibftpt.cpp:52:21: error: cast from ‘void*’ to ‘int’ loses
precision [-fpermissive]
int output = (int)fd;
^~
src/mgr/ftplibftpt.cpp:53:3: error: ‘write’ was not declared in this
scope
write(output, buffer, size);
^
src/mgr/ftplibftpt.cpp:53:3: error: ‘write’ was not declared in this
scope
write(output, buffer, size);
^
src/mgr/ftplibftpt.cpp:53:3: note: suggested alternative: ‘fwrite’
write(output, buffer, size);
See https://github.com/bibletime/bibletime/runs/1272250545 for the
failing build run.
Commit details in our git mirror of the Sword SVN repository:
https://github.com/bibletime/crosswire-sword-mirror/commit/c52559ecae
Best regards,
J
___
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
___
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
[sword-devel] SVN 3813 makes compilation of Sword fail
Hello!
The commit "A bit more work on making it easier to use SWORD in a
threadsafe manner." makes compilation of Sword fail:
src/mgr/ftplibftpt.cpp: In function ‘int
sword::{anonymous}::my_filewriter(netbuf*, void*, size_t, void*)’:
src/mgr/ftplibftpt.cpp:52:21: error: cast from ‘void*’ to ‘int’ loses
precision [-fpermissive]
int output = (int)fd;
^~
src/mgr/ftplibftpt.cpp: In function ‘int
sword::{anonymous}::my_filewriter(netbuf*, void*, size_t, void*)’:
src/mgr/ftplibftpt.cpp:52:21: error: cast from ‘void*’ to ‘int’ loses
precision [-fpermissive]
int output = (int)fd;
^~
src/mgr/ftplibftpt.cpp:53:3: error: ‘write’ was not declared in this scope
write(output, buffer, size);
^
src/mgr/ftplibftpt.cpp:53:3: error: ‘write’ was not declared in this scope
write(output, buffer, size);
^
src/mgr/ftplibftpt.cpp:53:3: note: suggested alternative: ‘fwrite’
write(output, buffer, size);
See https://github.com/bibletime/bibletime/runs/1272250545 for the
failing build run.
Commit details in our git mirror of the Sword SVN repository:
https://github.com/bibletime/crosswire-sword-mirror/commit/c52559ecae
Best regards,
J
___
sword-devel mailing list: sword-devel@crosswire.org
http://crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page
Re: [sword-devel] Cross references with Sword 1.9.0RC3
Gary, are you sure that the compiler did not error or at least warn you about this? Because strangely enough, in BibleTime, the overridden CSwordBackend::AddRenderFilter method already seems to have the override keyword present. I suspect what you saw might instead have been caused by something like compiling BibleTime against an older version of Sword, but running it against the newer version of the Sword dynamic library. This is a form of API breakage which might perhaps lead to results like this. If this is the case we might in the future get bug reports about this from downstream as well. J On 18.10.20 16:40, Jaak Ristioja wrote: Hi! The respective change in Sword is SVN 3753: https://github.com/bibletime/crosswire-sword-mirror/commit/b684b6f099 It seems that this time we were not lucky to have a SWDEPRECATED alias for the old function name, silently breaking API, and hence BibleTime. In theory this silent error could also have been detected in BibleTime if we would have used the override keyword. In practice, finding all the functions which need the override keyword is for most projects only easy to do if the -Wsuggest-override flag (and perhaps even -Werror=suggest-override for debug builds) is used. Unfortunately for BibleTime (and other projects using Sword), just turning this flag on yields very many warnings from Sword headers themselves. The task of figuring out how (and when in the build process) to filter BibleTime-specific warnings from all warnings is likely not a trivial one. Best regards, J On 18.10.20 01:50, Gary Holmlund wrote: I found the issue. BibleTime is using a sword function, AddRenderFilters, which is deprecated. It was renamed to addRenderFilters (A -> a). Gary On 10/17/20 1:36 PM, Gary Holmlund wrote: I have found that sword is not calling the BibleTime OsisToHtml filter. It is called with 1.8.1 and not with 1.9RC3. I am still working on why this does not happen. Gary On 10/16/20 3:49 AM, Troy A. Griffitts wrote: Hi Gary. No, nothing should have changed in that respect. Can anyone else confirm they are having trouble with these in our last RC or svn HEAD? On October 16, 2020 2:44:32 AM GMT+02:00, Gary Holmlund wrote: >I switched to sword 1.9 rc3 in BibleTime. I am on Fedora 33. Now I have > >lost cross references, footnotes, Jesus words, etc. > >Did any api for references change? Any other thoughts? > >Gary Holmlund > > >___ >sword-devel mailing list: sword-devel@crosswire.org >http://crosswire.org/mailman/listinfo/sword-devel >Instructions to unsubscribe/change your settings at above page -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page
Re: [sword-devel] Cross references with Sword 1.9.0RC3
Hi Jaak, Glad we found the issue during the RC stage. Yes, finishing the camelCase refactoring work for the 2.0 release was a large part of this release. We should have probably added a SWDEPRECATED flag to the method and left it, but for virtuals, this gets a bit tricky. For us to continue to support the old name, we need to be sure we still call any overridden version using the old name and of course call the new name, as well. I think we tried this in a few other earlier releases and it was a bit hairy-- including hushing our own deprecation violation when we called the old deprecated method name. With 2.0 as the likely next major release and thus all current SWDEPRECATED methods finally going away, it seemed best to just get clients off the deprecated methods now. I am glad we found the issue for BibleTime before release and I wish I had a better way for us to detect renaming of virtual methods. Using the new C++ keyword override is a great way for you to do this in your client; maybe you could grep out the sword methods somehow when using the compiler option to suggest places for the override keyword. Maybe we could add a SWOVERRIDE define to allow us to use it in the engine and still support compilers without support. Thank you for the suggestion. I believe we are done renaming virtuals for a very long time; I have no more refactorings like this on my to-do list and it's great to finally have all of these complete with this release. With Bibletime, Xiphos, Ezra, and Bishop all reporting in with positive results now, I believe we can feel good about pushing 1.9.0 out the door. I will likely wait until next weekend just to be sure we have peace on problem reports for a few days. Thank you and everyone for your help and support with this and for working together as the Body of Christ. It's a privilege and great fun to serve together with you guys. Troy On October 18, 2020 3:40:10 PM GMT+02:00, Jaak Ristioja wrote: >Hi! > >The respective change in Sword is SVN 3753: > > https://github.com/bibletime/crosswire-sword-mirror/commit/b684b6f099 > >It seems that this time we were not lucky to have a SWDEPRECATED alias >for the old function name, silently breaking API, and hence BibleTime. > >In theory this silent error could also have been detected in BibleTime >if we would have used the override keyword. In practice, finding all >the >functions which need the override keyword is for most projects only >easy >to do if the -Wsuggest-override flag (and perhaps even >-Werror=suggest-override for debug builds) is used. Unfortunately for >BibleTime (and other projects using Sword), just turning this flag on >yields very many warnings from Sword headers themselves. The task of >figuring out how (and when in the build process) to filter >BibleTime-specific warnings from all warnings is likely not a trivial >one. > >Best regards, >J > >On 18.10.20 01:50, Gary Holmlund wrote: >> I found the issue. BibleTime is using a sword function, >> AddRenderFilters, which is deprecated. It was renamed to >> addRenderFilters (A -> a). >> >> Gary >> >> On 10/17/20 1:36 PM, Gary Holmlund wrote: >>> I have found that sword is not calling the BibleTime OsisToHtml >>> filter. It is called with 1.8.1 and not with 1.9RC3. I am still >>> working on why this does not happen. >>> >>> Gary >>> >>> >>> On 10/16/20 3:49 AM, Troy A. Griffitts wrote: Hi Gary. No, nothing should have changed in that respect. Can >anyone else confirm they are having trouble with these in our last RC or >svn HEAD? On October 16, 2020 2:44:32 AM GMT+02:00, Gary Holmlund wrote: >I switched to sword 1.9 rc3 in BibleTime. I am on Fedora 33. Now I >have > >lost cross references, footnotes, Jesus words, etc. > >Did any api for references change? Any other thoughts? > >Gary Holmlund > > >___ >sword-devel mailing list: sword-devel@crosswire.org >http://crosswire.org/mailman/listinfo/sword-devel >Instructions to unsubscribe/change your settings at above page -- Sent from my Android device with K-9 Mail. Please excuse my >brevity. -- Sent from my Android device with K-9 Mail. Please excuse my >brevity. >> ___ >> sword-devel mailing list: sword-devel@crosswire.org >> http://crosswire.org/mailman/listinfo/sword-devel >> Instructions to unsubscribe/change your settings at above page > >___ >sword-devel mailing list: sword-devel@crosswire.org >http://crosswire.org/mailman/listinfo/sword-devel >Instructions to unsubscribe/change your settings at above page -- Sent from my Android device with K-9 Mail. Please excuse my brevity.___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel
Re: [sword-devel] Cross references with Sword 1.9.0RC3
Hi! The respective change in Sword is SVN 3753: https://github.com/bibletime/crosswire-sword-mirror/commit/b684b6f099 It seems that this time we were not lucky to have a SWDEPRECATED alias for the old function name, silently breaking API, and hence BibleTime. In theory this silent error could also have been detected in BibleTime if we would have used the override keyword. In practice, finding all the functions which need the override keyword is for most projects only easy to do if the -Wsuggest-override flag (and perhaps even -Werror=suggest-override for debug builds) is used. Unfortunately for BibleTime (and other projects using Sword), just turning this flag on yields very many warnings from Sword headers themselves. The task of figuring out how (and when in the build process) to filter BibleTime-specific warnings from all warnings is likely not a trivial one. Best regards, J On 18.10.20 01:50, Gary Holmlund wrote: I found the issue. BibleTime is using a sword function, AddRenderFilters, which is deprecated. It was renamed to addRenderFilters (A -> a). Gary On 10/17/20 1:36 PM, Gary Holmlund wrote: I have found that sword is not calling the BibleTime OsisToHtml filter. It is called with 1.8.1 and not with 1.9RC3. I am still working on why this does not happen. Gary On 10/16/20 3:49 AM, Troy A. Griffitts wrote: Hi Gary. No, nothing should have changed in that respect. Can anyone else confirm they are having trouble with these in our last RC or svn HEAD? On October 16, 2020 2:44:32 AM GMT+02:00, Gary Holmlund wrote: >I switched to sword 1.9 rc3 in BibleTime. I am on Fedora 33. Now I have > >lost cross references, footnotes, Jesus words, etc. > >Did any api for references change? Any other thoughts? > >Gary Holmlund > > >___ >sword-devel mailing list: sword-devel@crosswire.org >http://crosswire.org/mailman/listinfo/sword-devel >Instructions to unsubscribe/change your settings at above page -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page ___ sword-devel mailing list: sword-devel@crosswire.org http://crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page
