New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail with LOOLWSD on debian 9.2 #135

Closed
Gomez opened this Issue Nov 12, 2017 · 18 comments

Comments

Projects
None yet
9 participants
@Gomez
Contributor

Gomez commented Nov 12, 2017

I already tested many different branches / commits of lool.
Everytime it fails in this file: wsd/LOOLWSD.cpp.

Always the same error:

(CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/bash /opt/online/missing autoheader) rm -f stamp-h1 touch config.h.in cd . && /bin/bash ./config.status config.h config.status: creating config.h make all-recursive make[1]: Entering directory '/opt/online' Making all in . make[2]: Entering directory '/opt/online' CXX common/Crypto.o CXX wsd/Admin.o CXX wsd/AdminModel.o CXX wsd/Auth.o CXX wsd/DocumentBroker.o CXX wsd/LOOLWSD.o In file included from wsd/LOOLWSD.cpp:107:0: ./common/UnitHTTP.hpp:86:18: error: ‘virtual bool UnitHTTPServerRequest::expectContinue() const’ marked ‘override’, but does not override virtual bool expectContinue() const override ^~~~~~~~~~~~~~ Makefile:1185: recipe for target 'wsd/LOOLWSD.o' failed make[2]: *** [wsd/LOOLWSD.o] Error 1 make[2]: Leaving directory '/opt/online' Makefile:1735: recipe for target 'all-recursive' failed make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory '/opt/online' Makefile:748: recipe for target 'all' failed make: *** [all] Error 2
@embed-3d You have done the debian upgrade, you seen is before?
@aalaesar Any pointer other idea?

@Gomez

This comment has been minimized.

Show comment
Hide comment
@Gomez

Gomez Nov 12, 2017

Contributor

I tested with a jessie (Debian 8) VM and got the same error.

Contributor

Gomez commented Nov 12, 2017

I tested with a jessie (Debian 8) VM and got the same error.

@aalaesar

This comment has been minimized.

Show comment
Hide comment
@aalaesar

aalaesar Nov 12, 2017

Contributor

Looks like a issue with the gcc version.
This error seems to refer to advanced programming.
Found some hints over here
https://stackoverflow.com/questions/29145476/requiring-virtual-function-overrides-to-use-override-keyword

Maybe there is a option to turn down this error.
Best luck
Aal

Contributor

aalaesar commented Nov 12, 2017

Looks like a issue with the gcc version.
This error seems to refer to advanced programming.
Found some hints over here
https://stackoverflow.com/questions/29145476/requiring-virtual-function-overrides-to-use-override-keyword

Maybe there is a option to turn down this error.
Best luck
Aal

@aalaesar

This comment has been minimized.

Show comment
Hide comment
@aalaesar

aalaesar Nov 12, 2017

Contributor

Didn't saw your response, could be a bug introduced. In lo
Do you use latest commit or use release tags ?

Contributor

aalaesar commented Nov 12, 2017

Didn't saw your response, could be a bug introduced. In lo
Do you use latest commit or use release tags ?

@embed-3d

This comment has been minimized.

Show comment
Hide comment
@embed-3d

embed-3d Nov 13, 2017

Contributor

I got it working a few weeks ago, without any problems. It is the first time I see this error.
The error message looks for me more like a bug in loolwsd.

./common/UnitHTTP.hpp:86:18: error: ‘virtual bool UnitHTTPServerRequest::expectContinue() const’ marked ‘override’, but does not override virtual bool expectContinue() const override

Since the class is inherited from a Poco and a couldn't see there any changes related to that in loolwsd you can try to use an older libPoco version.

class UnitHTTPServerRequest : public Poco::Net::HTTPServerRequest

@aalaesar We should think about checking out specific version tags, to make sure to get always a stable build with the default settings.

Contributor

embed-3d commented Nov 13, 2017

I got it working a few weeks ago, without any problems. It is the first time I see this error.
The error message looks for me more like a bug in loolwsd.

./common/UnitHTTP.hpp:86:18: error: ‘virtual bool UnitHTTPServerRequest::expectContinue() const’ marked ‘override’, but does not override virtual bool expectContinue() const override

Since the class is inherited from a Poco and a couldn't see there any changes related to that in loolwsd you can try to use an older libPoco version.

class UnitHTTPServerRequest : public Poco::Net::HTTPServerRequest

@aalaesar We should think about checking out specific version tags, to make sure to get always a stable build with the default settings.

@aalaesar

This comment has been minimized.

Show comment
Hide comment
@aalaesar

aalaesar Nov 13, 2017

Contributor

@aalaesar We should think about checking out specific version tags, to make sure to get always a stable build with the default settings.

That should be already the case.
by default the script choose the latest tag for selected branches.
(The way the project make tags seems to be 1 tag by release or prerelease)

Contributor

aalaesar commented Nov 13, 2017

@aalaesar We should think about checking out specific version tags, to make sure to get always a stable build with the default settings.

That should be already the case.
by default the script choose the latest tag for selected branches.
(The way the project make tags seems to be 1 tag by release or prerelease)

@Gomez

This comment has been minimized.

Show comment
Hide comment
@Gomez

Gomez Nov 13, 2017

Contributor

Thanks for the pointers!

I solved it by setting this in the officeonline-install.cfg:

#### POCO parameters ###
poco_version_latest='1.7.9p2'  ##  fetched from the poco project
poco_version=$poco_version_latest
poco_dir="/opt/poco-${poco_version}-all"
#poco_forcebuild=false
poco_version_folder='1.7.9'  ##  fetched from the poco project
#poco_req_vol=550 # minimum space required for Poco compilation, in MB
#

As pointed out by the libreoffice devs it could be a easy fix (haven't tested yet):

[11:52:16] <_rene_> # grep VERSION Version.h
[11:52:17] <_rene_> #define POCO_VERSION 0x01080000
[11:54:06] <tml__> ok, so s/0x02000000/0x01080000 probably helps

I agree with @embed-3d we should use specific versions. In this case 1.7.9 of poco, as it is the last working version. If poco 1.8.0 works (tested by someone) we create a PR and update it.

Contributor

Gomez commented Nov 13, 2017

Thanks for the pointers!

I solved it by setting this in the officeonline-install.cfg:

#### POCO parameters ###
poco_version_latest='1.7.9p2'  ##  fetched from the poco project
poco_version=$poco_version_latest
poco_dir="/opt/poco-${poco_version}-all"
#poco_forcebuild=false
poco_version_folder='1.7.9'  ##  fetched from the poco project
#poco_req_vol=550 # minimum space required for Poco compilation, in MB
#

As pointed out by the libreoffice devs it could be a easy fix (haven't tested yet):

[11:52:16] <_rene_> # grep VERSION Version.h
[11:52:17] <_rene_> #define POCO_VERSION 0x01080000
[11:54:06] <tml__> ok, so s/0x02000000/0x01080000 probably helps

I agree with @embed-3d we should use specific versions. In this case 1.7.9 of poco, as it is the last working version. If poco 1.8.0 works (tested by someone) we create a PR and update it.

@aalaesar

This comment has been minimized.

Show comment
Hide comment
@aalaesar

aalaesar Nov 13, 2017

Contributor

I agree with @embed-3d we should use specific versions. In this case 1.7.9 of poco, as it is the last working version. If poco 1.8.0 works (tested by someone) we create a PR and update it.

Hi
I'm a lazy person ( 😃) so I would say that the script is working as intended because:
1- that's the goal of the external config file (that can be used from outside of the git project for recall): to adjust parameters depending of the environments and/or choices made by users.
2- This would be a fix that break automation for only a temporary problem.
3- This would requires works and more support. (keeping things up to date) ... and adds commits with somewhat no value.
4- Let's keep the conf away from the code at maximum. Let the user manage it. Default is just for optimal use case.

If you agree, I would conclude:
I'm glad you got this issue sorted 👍 let's just move on ⛵️

Ps: I'm also completely ok with the fact that's I might be wrong and the problem may persist. In that case a patch may be needed.

Contributor

aalaesar commented Nov 13, 2017

I agree with @embed-3d we should use specific versions. In this case 1.7.9 of poco, as it is the last working version. If poco 1.8.0 works (tested by someone) we create a PR and update it.

Hi
I'm a lazy person ( 😃) so I would say that the script is working as intended because:
1- that's the goal of the external config file (that can be used from outside of the git project for recall): to adjust parameters depending of the environments and/or choices made by users.
2- This would be a fix that break automation for only a temporary problem.
3- This would requires works and more support. (keeping things up to date) ... and adds commits with somewhat no value.
4- Let's keep the conf away from the code at maximum. Let the user manage it. Default is just for optimal use case.

If you agree, I would conclude:
I'm glad you got this issue sorted 👍 let's just move on ⛵️

Ps: I'm also completely ok with the fact that's I might be wrong and the problem may persist. In that case a patch may be needed.

@ExaconAT

This comment has been minimized.

Show comment
Hide comment
@ExaconAT

ExaconAT Nov 15, 2017

POCO parameters

poco_version_latest='1.7.9p2' ## fetched from the poco project
poco_version=$poco_version_latest
poco_dir="/opt/poco-${poco_version}-all"
#poco_forcebuild=false
poco_version_folder='1.7.9' ## fetched from the poco project
#poco_req_vol=550 # minimum space required for Poco compilation, in MB

Loolwsd install had an error, coldnt connect to webserver... Trying the 3rd time now.

ExaconAT commented Nov 15, 2017

POCO parameters

poco_version_latest='1.7.9p2' ## fetched from the poco project
poco_version=$poco_version_latest
poco_dir="/opt/poco-${poco_version}-all"
#poco_forcebuild=false
poco_version_folder='1.7.9' ## fetched from the poco project
#poco_req_vol=550 # minimum space required for Poco compilation, in MB

Loolwsd install had an error, coldnt connect to webserver... Trying the 3rd time now.

@Kassiematis

This comment has been minimized.

Show comment
Hide comment
@Kassiematis

Kassiematis Nov 24, 2017

Contributor

Seems that with collabora online version 3-0 problem with poco is fixed.

LibreOffice/online@623b45a

Contributor

Kassiematis commented Nov 24, 2017

Seems that with collabora online version 3-0 problem with poco is fixed.

LibreOffice/online@623b45a

@keros

This comment has been minimized.

Show comment
Hide comment
@keros

keros Nov 27, 2017

Build it today on Ubuntu 16.04
Without the extra poco lines in the "officeonline-install.cfg" the build fails

keros commented Nov 27, 2017

Build it today on Ubuntu 16.04
Without the extra poco lines in the "officeonline-install.cfg" the build fails

@Kassiematis

This comment has been minimized.

Show comment
Hide comment
@Kassiematis

Kassiematis Nov 27, 2017

Contributor

Which version of Loolwsd is installed? I have changed officeonline-install.cfg

set_online_regex='collabora-online-3-0'

The rest is standard.

Contributor

Kassiematis commented Nov 27, 2017

Which version of Loolwsd is installed? I have changed officeonline-install.cfg

set_online_regex='collabora-online-3-0'

The rest is standard.

@keros

This comment has been minimized.

Show comment
Hide comment
@keros

keros Nov 27, 2017

I used the install script out of the box with no config.
So I think not 3.0?
Seems that the default version of Loolwsd currently is not 3.0

keros commented Nov 27, 2017

I used the install script out of the box with no config.
So I think not 3.0?
Seems that the default version of Loolwsd currently is not 3.0

@czebiniak

This comment has been minimized.

Show comment
Hide comment
@czebiniak

czebiniak Dec 6, 2017

My 2 cents would be, if the end result is a consistent failure across the board for all using the script, then at least include a fallback option that users can rely on if something like poco fails. Not sure how easy it would be to implement this, but I am trying to run this on 4 different machines and all failed with Ubuntu 16.04 with the exact same error as stated above. Fixing the Poco config fixed my issue.

czebiniak commented Dec 6, 2017

My 2 cents would be, if the end result is a consistent failure across the board for all using the script, then at least include a fallback option that users can rely on if something like poco fails. Not sure how easy it would be to implement this, but I am trying to run this on 4 different machines and all failed with Ubuntu 16.04 with the exact same error as stated above. Fixing the Poco config fixed my issue.

@aalaesar

This comment has been minimized.

Show comment
Hide comment
@aalaesar

aalaesar Dec 6, 2017

Contributor

Hi I suppose a warning in the readme could be a start ? Or a dedicated file about known issues ?
Actually this is very complicated to implement an automatic fallback : as the script is designed with maximum flexibility for the user in mind. Starting to manage integration issues between projects add a full new level of cases to create and support.

Contributor

aalaesar commented Dec 6, 2017

Hi I suppose a warning in the readme could be a start ? Or a dedicated file about known issues ?
Actually this is very complicated to implement an automatic fallback : as the script is designed with maximum flexibility for the user in mind. Starting to manage integration issues between projects add a full new level of cases to create and support.

@quenenni

This comment has been minimized.

Show comment
Hide comment
@quenenni

quenenni Dec 17, 2017

Hello. My turn to show you my last experiences..

Lots of attemps to finally find what commit in libreoffice core introduced the error (the same I saw on other posts here:

/opt/libreoffice2/vcl/null/printerinfomanager.cxx: In constructor ‘psp::PrinterInfoManager::PrinterInfoManager(psp::PrinterInfoManager::Type)’:
/opt/libreoffice2/vcl/null/printerinfomanager.cxx:47:33: error: call of overloaded ‘unique_ptr(NULL)’ is ambiguous
     m_aSystemDefaultPaper( "A4" )
...

The commit that brought this error is this one:
LibreOffice/core@0f455f2

So, a git checkout 07ff6eaaba5a7ecd0e9e04507190df779ac5cfe4 (the one just before) before compiling should make it work.

After that, I had also to select the 1.7.9p2 ver of Poco coz 1.8.01 breaks the compilation of online (I didn't try with v1.8.0).
And I let the git commit to the online repo the default selected by the script.

I have a lool nearly functionning.
The general view, the buttons and the layout show.
The data is there, but not visible. In odt file, if I go in the small box where you have the coordinates of the selected cell (top left), focus on it and hit 'Enter', then you can afterwards move the cell selection (with the arrow keys i.e.) and when you move it on a cell that has data, the data is showed in the address bar.
And I can change it, and it saves the changes.

Sooo.. Nearly there.

The repeated error I see in my logs:
kit-02262-02270 12:31:39.502281 [ lokit_001 ] ERR Tile rendering requested without views.| kit/Kit.cpp:1005

I hope this will bring some lights for someone.

PS: @aalaesar I tried using the var lo_src_commit='' (# the short/full id of a git commit) to select the commit I wanted but it generates an error saying the commit doesn't exist.
I think it's because the script tries to git checkout ze_commit before selecting the correct branch.
So it tries on the master branch and of course, there, that commit doesn't exist.

quenenni commented Dec 17, 2017

Hello. My turn to show you my last experiences..

Lots of attemps to finally find what commit in libreoffice core introduced the error (the same I saw on other posts here:

/opt/libreoffice2/vcl/null/printerinfomanager.cxx: In constructor ‘psp::PrinterInfoManager::PrinterInfoManager(psp::PrinterInfoManager::Type)’:
/opt/libreoffice2/vcl/null/printerinfomanager.cxx:47:33: error: call of overloaded ‘unique_ptr(NULL)’ is ambiguous
     m_aSystemDefaultPaper( "A4" )
...

The commit that brought this error is this one:
LibreOffice/core@0f455f2

So, a git checkout 07ff6eaaba5a7ecd0e9e04507190df779ac5cfe4 (the one just before) before compiling should make it work.

After that, I had also to select the 1.7.9p2 ver of Poco coz 1.8.01 breaks the compilation of online (I didn't try with v1.8.0).
And I let the git commit to the online repo the default selected by the script.

I have a lool nearly functionning.
The general view, the buttons and the layout show.
The data is there, but not visible. In odt file, if I go in the small box where you have the coordinates of the selected cell (top left), focus on it and hit 'Enter', then you can afterwards move the cell selection (with the arrow keys i.e.) and when you move it on a cell that has data, the data is showed in the address bar.
And I can change it, and it saves the changes.

Sooo.. Nearly there.

The repeated error I see in my logs:
kit-02262-02270 12:31:39.502281 [ lokit_001 ] ERR Tile rendering requested without views.| kit/Kit.cpp:1005

I hope this will bring some lights for someone.

PS: @aalaesar I tried using the var lo_src_commit='' (# the short/full id of a git commit) to select the commit I wanted but it generates an error saying the commit doesn't exist.
I think it's because the script tries to git checkout ze_commit before selecting the correct branch.
So it tries on the master branch and of course, there, that commit doesn't exist.

@aalaesar

This comment has been minimized.

Show comment
Hide comment
@aalaesar

aalaesar Dec 17, 2017

Contributor

Hi @quenenni
thank you for this feedback 👍
The build error has been "fixed" (much more like a work around) with the latest pull request

PS: @aalaesar I tried using the var lo_src_commit='' (# the short/full id of a git commit) to select the commit I wanted but it generates an error saying the commit doesn't exist.
I think it's because the script tries to git checkout ze_commit before selecting the correct branch.
So it tries on the master branch and of course, there, that commit doesn't exist.

Thanks 👍 This seems to be a design flaw is the SearGitCommit function that need to be fixed.
I'll look for a fix soon.

Regards, Aal

Contributor

aalaesar commented Dec 17, 2017

Hi @quenenni
thank you for this feedback 👍
The build error has been "fixed" (much more like a work around) with the latest pull request

PS: @aalaesar I tried using the var lo_src_commit='' (# the short/full id of a git commit) to select the commit I wanted but it generates an error saying the commit doesn't exist.
I think it's because the script tries to git checkout ze_commit before selecting the correct branch.
So it tries on the master branch and of course, there, that commit doesn't exist.

Thanks 👍 This seems to be a design flaw is the SearGitCommit function that need to be fixed.
I'll look for a fix soon.

Regards, Aal

@aalaesar

This comment has been minimized.

Show comment
Hide comment
@aalaesar

aalaesar Dec 17, 2017

Contributor

@quenenni #146 fix the issue with lo_src_commit

Contributor

aalaesar commented Dec 17, 2017

@quenenni #146 fix the issue with lo_src_commit

@quenenni

This comment has been minimized.

Show comment
Hide comment
@quenenni

quenenni Dec 18, 2017

That was quick. Thanks!

It's fixed.. Nice.. I'll give it a try soon.. well asap.

quenenni commented Dec 18, 2017

That was quick. Thanks!

It's fixed.. Nice.. I'll give it a try soon.. well asap.

@husisusi husisusi closed this Jan 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment