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

Remove check on Qt version #2022

Merged
merged 4 commits into from
Jul 11, 2023
Merged

Remove check on Qt version #2022

merged 4 commits into from
Jul 11, 2023

Conversation

sarlinpe
Copy link
Contributor

I'm on Ubuntu 18.04 and seeing the extra -fPIC flag added by Qt 5.9.5, as reported in #1753 #1813. Upgrading to cmake 3.26 does not help. I think that it's safer to remove the flag regardless of the Qt version.

@ahojnnes
Copy link
Contributor

Are you stuck with 18.04? I was planning to not support 18.04 going forward, as its outdated default compiler/packages hold development back due to lack of C++14, etc.

@sarlinpe
Copy link
Contributor Author

sarlinpe commented Jul 10, 2023

Nope I indeed need to update very soon... But I don't think that this issue is specific to Ubuntu 18.04.

@ahojnnes
Copy link
Contributor

I would rather add the 5.9.5 version explicitly, as it does appear to be fixed for the Qt version under 20.04 and 22.04. It sounds a little dangerous to mess with the default flags across all versions.

@ahojnnes ahojnnes enabled auto-merge (squash) July 11, 2023 06:47
@ahojnnes
Copy link
Contributor

ahojnnes commented Jul 11, 2023

Looked at the cmake bug again and also the special casing here. I think we can go ahead as proposed by you. Apologies , didn't have time to look with necessary detail yesterday. Thanks.

auto-merge was automatically disabled July 11, 2023 07:33

Head branch was pushed to by a user without write access

@ahojnnes ahojnnes merged commit 2ea1631 into colmap:dev Jul 11, 2023
11 checks passed
@sarlinpe sarlinpe deleted the fix-qt-fpic branch February 8, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants