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.

Sign up for GitHub

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

Jump to bottom

config: Create dynamic library of C++ libraries #2612

Merged
merged 2 commits into from
Nov 12, 2022
Merged

config: Create dynamic library of C++ libraries #2612

merged 2 commits into from
Nov 12, 2022

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Oct 26, 2022

Up till now linking C++ based libraries (at this point only the iostream library) was forced into creating a static library. This PR enables the possibility to create a dynamic library with the new configure flag 'SHLIB_LDX', which could use CXX instead of CC.

IS_CXX = yes must be set before including the 'Lib.make' file in the library's Makefile.

(retry of #2611 )

@nilason
lib config: create dynamic library of C++ libraries
6040dc1 
Up till now linking C++ based libraries (at this point only the
'iostream' library) was forced into creating a static library.
This enables the possibility to create a dynamic library with the
new configure flag 'SHLIB_LDX', which could use CXX instead of CC.

Make sure to set 'IS_CXX = yes' before including the 'Lib.make' file
in the library's Makefile.
@nilason
earlier CXX detection
ab9a3d2
@wenzeslaus
Copy link
Member

Maybe one of the GCC matrix configs should test this.

BTW, I see an email from GitHub for ab9a3d2 for the other PR, but I don't know what went wrong on the web.

@nilason
Copy link
Contributor Author

nilason commented Oct 26, 2022

Maybe one of the GCC matrix configs should test this.

I believe all will create dynamic libs by default, e.g.:

...
g++ -shared -o /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib/libgrass_iostream.8.3.so -L/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib -L/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib -Wl,--export-dynamic -Wl,-rpath-link,/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib -Wl,-rpath,/home/runner/install/grass83/lib -Wl,-soname,libgrass_iostream.8.3.so OBJ.x86_64-pc-linux-gnu/ami_stream.o OBJ.x86_64-pc-linux-gnu/mm.o OBJ.x86_64-pc-linux-gnu/mm_utils.o OBJ.x86_64-pc-linux-gnu/rtimer.o  -lgrass_gis.8.3 -lm 
...

BTW, I see an email from GitHub for ab9a3d2 for the other PR, but I don't know what went wrong on the web.

I have no clue what happened, or rather why nothing happened, I did what I always do.

@nilason
Copy link
Contributor Author

nilason commented Oct 26, 2022 •
edited
Loading

This is what the previously mentioned log line looked like before:

ar cr /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib/libgrass_iostream.8.3.a OBJ.x86_64-pc-linux-gnu/ami_stream.o OBJ.x86_64-pc-linux-gnu/mm.o OBJ.x86_64-pc-linux-gnu/mm_utils.o OBJ.x86_64-pc-linux-gnu/rtimer.o; ranlib /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib/libgrass_iostream.8.3.a

@nilason
Copy link
Contributor Author

nilason commented Oct 26, 2022

@lbartoletti If you have the possibility to test if this PR causes any regression on BSD systems, I'd be grateful.

@lbartoletti
Copy link
Contributor

@lbartoletti If you have the possibility to test if this PR causes any regression on BSD systems, I'd be grateful.

I successfully build your branch.

Thanks!

@nilason
Copy link
Contributor Author

nilason commented Oct 26, 2022

@lbartoletti If you have the possibility to test if this PR causes any regression on BSD systems, I'd be grateful.

I successfully build your branch.

Thanks!

I figured it would work, but one never knows... Thanks for taking the time!!

@nilason
Copy link
Contributor Author

nilason commented Oct 26, 2022

To summarise the results from testing:
The configure flag --enable-shared which is set by default, now works also for creating C++ GRASS libraries like libgrass_iostream.
There are two modules (r.terraflow and r.viewshed) linking to the libgrass_iostream library, which are both built and tested successfully on the CI Windows, CI Ubuntu, MacOS, and BSD.

@nilason nilason added this to the 8.3.0 milestone Nov 4, 2022
@nilason
Copy link
Contributor Author

nilason commented Nov 9, 2022

@sebastic Do you have any thoughts or objections to this?

@sebastic
Copy link
Contributor

sebastic commented Nov 9, 2022

No. As long as GRASS doesn't adhere to FHS, all libraries are installed outside the ld search paths and considered private.

@nilason nilason merged commit 0e0d23f into OSGeo:main Nov 12, 2022
@nilason nilason deleted the shlib_with_cxx branch November 14, 2022 11:15
jef-n added a commit to jef-n/OSGeo4W that referenced this pull request Nov 23, 2022
@jef-n
'fix' grass-dev build (was not building at all)
2d3c9fb 
Now running into an upstream problem (maybe caused by OSGeo/grass#2612):

make[4]: *** No rule to make target '…/grass/dist.x86_64-w64-mingw32/lib/libgrass_iostream.8.3.dll', needed by '…/grass/dist.x86_64-w64-mingw32/bin/r.terraflow.exe'.
make[4]: *** No rule to make target '…/grass/dist.x86_64-w64-mingw32/lib/libgrass_iostream.8.3.dll', needed by '…/grass/dist.x86_64-w64-mingw32/bin/r.viewshed.exe'.
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@nilason @ninsbl
lib config: create dynamic library of C++ libraries (OSGeo#2612)
63f1ef8 
Up till now linking C++ based libraries (at this point only the
'iostream' library) was forced into creating a static library.
This enables the possibility to create a dynamic library with the
new configure flag 'SHLIB_LDX', which could use CXX instead of CC.

Note: Make sure to set 'IS_CXX = yes' before including the 'Lib.make'
file in the library's Makefile.
marisn pushed a commit to marisn/grass that referenced this pull request Jun 2, 2023
@nilason @marisn
lib config: create dynamic library of C++ libraries (OSGeo#2612)
6571e93 
Up till now linking C++ based libraries (at this point only the
'iostream' library) was forced into creating a static library.
This enables the possibility to create a dynamic library with the
new configure flag 'SHLIB_LDX', which could use CXX instead of CC.

Note: Make sure to set 'IS_CXX = yes' before including the 'Lib.make'
file in the library's Makefile.
@wenzeslaus wenzeslaus changed the title lib config: create dynamic library of C++ libraries (2nd attempt) config: Create dynamic library of C++ libraries Jun 6, 2023
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
@nilason @neteler
lib config: create dynamic library of C++ libraries (OSGeo#2612)
39eeda8 
Up till now linking C++ based libraries (at this point only the
'iostream' library) was forced into creating a static library.
This enables the possibility to create a dynamic library with the
new configure flag 'SHLIB_LDX', which could use CXX instead of CC.

Note: Make sure to set 'IS_CXX = yes' before including the 'Lib.make'
file in the library's Makefile.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
8.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@nilason @wenzeslaus @lbartoletti @sebastic