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

use already installed FindGeographicLib.cmake #1775

Merged
merged 2 commits into from
Aug 23, 2022
Merged

use already installed FindGeographicLib.cmake #1775

merged 2 commits into from
Aug 23, 2022

Conversation

acxz
Copy link
Contributor

@acxz acxz commented Aug 21, 2022

Solve #1774

The FindGeographicLib.cmake is already shipped with the distribution through the geographiclib package, hence our custom one (which seems to fail for later geographiclib versions) is not needed anymore.

Note: for debian based systems, FindGeographicLib.cmake is installed elsewhere, this path would needed to be added to the paths that find_package searches through.

I know that primary focus has been shifted to the ros2 branch, so let me know if I should make another PR to that branch. And if so, that commit would also close #1760

@vooon
Copy link
Member

vooon commented Aug 22, 2022

I'd prefer to keep existing module, but try to use package one's first.
There are still lots of os versions with kinetic, where i'd prefer to keep possibility to build master.

@acxz
Copy link
Contributor Author

acxz commented Aug 22, 2022

@vooon The module offered by the package is included in kinetic as well.

This link shows that the module is located in bionic. Since kinetic is EOL, Ubuntu doesn't have the package database for xenial.

Considering that xenial was released in April 21st, 2016 and referencing that data with ubuntu's package archives for the libgeographic-dev package. I took the package version (date release: 2015-08-19) that was created before the release of xenial and extracted to see that the cmake module file FindGeographicLib.cmake does exist.

This commit does build on kinetic with master.

@vooon
Copy link
Member

vooon commented Aug 22, 2022

Just time-to-time i have questions about old releases, on old raspbian or about old hardware.
That's why i'd like to keep possibility to build there.

But the same time i'm okay to drop deprecated things.

@acxz
Copy link
Contributor Author

acxz commented Aug 22, 2022

That's why i'd like to keep possibility to build there.

For sure, but even if we wanted to we couldn't bloom mavros into a version older than melodic anyway, so any user wouldn't have even have access to this commit. And even if they did want to compile the master branch onto unsupported ros versions, it would still work back to ROS Indigo (ubuntu 14) released in 2014. (I can check further if you want).

There is most likely going to be another compile issue that pops before this issue does when trying to compile the master branch for hardware/operating systems 8 years ago.

But the same time i'm okay to drop deprecated things.

I don't think the FindGeographicLib.cmake that we have is deprecated, it just doesn't stay updated since we maintain it ourselves. Increasing the codebase burden we have to maintain. So instead of using upstream's FindGeographicLib.cmake which will work for whatever distro you are on, by maintaining it ourselves, we have to keep the burden of keeping a single FindGeographicLib.cmake that works for all the distros we want.

@vooon
Copy link
Member

vooon commented Aug 23, 2022

@acxz okay. Thanks for the contribution!

@vooon vooon merged commit d959d63 into mavlink:master Aug 23, 2022
okalachev added a commit to CopterExpress/clover that referenced this pull request Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants