Skip to content
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

Error in Installing JSBSim Python module from source #379

Closed
viren3999 opened this issue Jan 22, 2021 · 16 comments
Closed

Error in Installing JSBSim Python module from source #379

viren3999 opened this issue Jan 22, 2021 · 16 comments

Comments

@viren3999
Copy link

viren3999 commented Jan 22, 2021

Hello.

I am working on an x64 Windows machine.

I have the latest stable version of Python (64-bit) and Visual Studio 2017 installed on my PC.

I wish to build and install the JSBSim Python module from the source code itself (to keep up with the latest commits).

I have built the JSBSim.sln in Release - x64 mode. (Output: 23 succeeded, 0 failed, 5 skipped)

The following commands were suggested by @seanmcleod to be executed to install the python module successfully.

  • python python/setup.py bdist_wheel

  • pip install jsbsim --no-index -f python/dist

However, when running python python/setup.py bdist_wheel in the build directory, I get the following error.

error: Don't know how to compile src/GeographicLib/Constants.hpp

image

@bcoconni
Copy link
Member

Confirmed, I can reproduce the bug locally.

bcoconni added a commit that referenced this issue Jan 23, 2021
This is following the issue reported #379. This change updates the CI workflow to test the build from source. The process fails as reported by #379.
bcoconni added a commit that referenced this issue Jan 23, 2021
Fixes issue #379 except for MacOSX which still fails.
@bcoconni
Copy link
Member

This should now be fixed following the commit be0d76f. For the record, the regression happened when the library GeographicLib was included in JSBSim 3 months ago (commit 70a327f). GeographicLib introduces a new extension for C++ headers .hpp which was not accounted for in the build script of our Python module, hence the error you got.

Also while testing for the bug you reported, I discovered that the build also failed on MacOSX for another reason (fixed by commit 9104aed).

So I took this opportunity to modify our CI workflow to check that the build from source works for Windows, MacOSX and Linux. We should now be warned earlier if a new regression is introduced.

Could you please check and report if the issue is fixed for you too ?

Thanks.

bcoconni added a commit that referenced this issue Jan 24, 2021
This is following the issue reported #379. This change updates the CI workflow to test the build from source. The process fails as reported by #379.
bcoconni added a commit that referenced this issue Jan 24, 2021
Fixes issue #379 except for MacOSX which still fails.
@viren3999
Copy link
Author

Could you please check and report if the issue is fixed for you too ?

@bcoconni well, I am facing a new error now. 😔

When I run python python/setup.py bdist_wheel

It gives the following error:
image

Is this syntax error specific to MSVC?

@seanmcleod
Copy link
Member

@viren3999 pet peeve of mine 😉 Rather than posting an image of your console output, paste it as simple text, i.e. copy the text from the console window and paste it into say a shell formatted block, e.g.

FGJSBBase.cpp
src/FGJSBBase.cpp(78): error C2017: illegal escape sequence

This means the text from your console output will be searchable by search engines, github etc. in future if someone does a search with some of the same error text etc.

Now in terms of your specific error it looks like there is an issue generating the version string in the source code.

const string FGJSBBase::JSBSim_version = JSBSIM_VERSION " " __DATE__ " " __TIME__ ;

@viren3999
Copy link
Author

viren3999 commented Jan 25, 2021

@viren3999 pet peeve of mine 😉 Rather than posting an image of your console output, paste it as simple text, i.e. copy the text from the console window and paste it into say a shell formatted block, e.g.

This means the text from your console output will be searchable by search engines, github etc. in future if someone does a search with some of the same error text etc.

@seanmcleod Sure Sir. Will keep it in mind hereafter.

Now in terms of your specific error it looks like there is an issue generating the version string in the source code

But when building JSBSim Windows executable, no such error occurs, despite the compiler being MSVC 🤔

@seanmcleod
Copy link
Member

But when building JSBSim Windows executable...

I'm guessing when you build the Windows executable you're using the supplied Solution/Project file? If so you'll notice that JSBSIM_VERSION is defined in the project file.

image

I'm not that familiar with the build process used during python python/setup.py bdist_wheel but my guess is that JSBSIM_VERSION is ending up with some invalid characters.

@seanmcleod
Copy link
Member

BTW the hard-code JSBSIM_VERSION in the project file isn't ideal.

Now the CMake based build system, which I'm guessing is what is being used by python python/setup.py bdist_wheel defines JSBSIM_VERSION in src\CmakeLists.txt.

add_definitions(-DJSBSIM_VERSION="${PROJECT_VERSION}${VERSION_MESSAGE}")

@bcoconni
Copy link
Member

@viren3999 What is exactly the context in which you run python python/setup.py bdist_wheel ? I mean what is the exact sequence of commands that you ran before executing python python/setup.py bdist_wheel ?

What puzzles me is:

  • bdist_wheel triggers a build from source. This should not happen if you've run make beforehand.
  • python/setup.py is created by the cmake command which automatically populates python/setup.py with relevant values (including JSBSIM_VERSION) for a successful build. So the build should not fail with that error.

Could you please check that line 186 in python/setup.py contains the following ?

        compile_flags = ['-D'+flag for flag in ['JSBSIM_VERSION="1.2.0.dev1"','HAVE_EXPAT_CONFIG_H',]]

@seanmcleod
Copy link
Member

bdist_wheel triggers a build from source. This should not happen if you've run make beforehand.

I have built the JSBSim.sln in Release - x64 mode. (Output: 23 succeeded, 0 failed, 5 skipped)

So the issue is that cmake make aren't being used to build JSBSim before bdist_wheel, rather a parallel build process using the Visual Studio solution/project file.

@bcoconni
Copy link
Member

So the issue is that cmake make aren't being used to build JSBSim before bdist_wheel, rather a parallel build process using the Visual Studio solution/project file.

OK so that would explain why bdist_wheel is building from sources. But still, the Visual Studio solution/project does not build python/setup.py, does it ? So cmake must have been run somehow and JSBSIM_VERSION set to correct value in the process ?

@seanmcleod
Copy link
Member

Correct, the Visual Studio solution/project, as opposed to using Visual Studio with cmake, doesn't build python\setup.py. If I look in my repo locally I only have python\setup.py.in after building using the Visual Studio project file.

@bcoconni
Copy link
Member

Correct, the Visual Studio solution/project, as opposed to using Visual Studio with cmake, doesn't build python\setup.py. If I look in my repo locally I only have python\setup.py.in after building using the Visual Studio project file.

Which makes me realize that the discussion has switched from building JSBSim on an Ubuntu virtual machine (issue #373) to a Windows machine (current issue).

So the exact command to build the Python module on Windows is

C:\> python python\setup.py --config RelWithDebInfo

The argument --config is used to tell setup.py where to locate the library libJSBSim.a that has been built by MSVC. The supplied parameter should be one of Debug, Release, RelDebWithInfo depending on which option was enabled when JSBSim was built by MSVC.

If --config is not supplied then setup.py fails to locate libJSBSim.a and therefore triggers a build from source. Given that multitasking with Python is not an easy subject (to say the least), setup.py does not parallelize compilation. As a result, building the Python module from sources with setup.py is taking more time than building the static library with MSVC first then calling setup.py with --config.

This does not solve the JSBSIM_VERSION flag mystery however.

@bcoconni
Copy link
Member

Ping @viren3999. Any progress on this issue ?

@viren3999
Copy link
Author

Ping @viren3999. Any progress on this issue ?

Unfortunately, no @bcoconni .
I am instead using the stable release through pypi for my usage !

@bcoconni
Copy link
Member

Unfortunately, no @bcoconni .
I am instead using the stable release through pypi for my usage !

If you no longer consider building the Python package yourself then I will close this issue since our CI workflow indicates that building from sources works again. Any objection ?

@viren3999
Copy link
Author

No objections Sir.
Thnx for the support 😄

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

No branches or pull requests

3 participants