Skip to content

wrap QMatrix in namespace pcl to resolve linker conflict with Qt6#4858

Merged
kunaltyagi merged 1 commit intoPointCloudLibrary:masterfrom
maz-1:master
Jul 27, 2021
Merged

wrap QMatrix in namespace pcl to resolve linker conflict with Qt6#4858
kunaltyagi merged 1 commit intoPointCloudLibrary:masterfrom
maz-1:master

Conversation

@maz-1
Copy link
Copy Markdown
Contributor

@maz-1 maz-1 commented Jul 21, 2021

fixes conflict with QtGui. Resolves #4857

@larshg
Copy link
Copy Markdown
Contributor

larshg commented Jul 21, 2021

fixes conflict with QtGui
Copy link
Copy Markdown
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Copy link
Copy Markdown
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably also wrap the Kernel class 😆

@kunaltyagi kunaltyagi added this to the pcl-1.12.1 milestone Jul 27, 2021
@kunaltyagi kunaltyagi merged commit 4a073c1 into PointCloudLibrary:master Jul 27, 2021
@kunaltyagi kunaltyagi added the changelog: fix Meta-information for changelog generation label Jul 27, 2021
@kunaltyagi kunaltyagi changed the title wrap QMatrix in namespace pcl wrap QMatrix in namespace pcl to resolve linker conflict Jul 27, 2021
@kunaltyagi kunaltyagi added the changelog: ABI break Meta-information for changelog generation label Dec 10, 2021
@kunaltyagi kunaltyagi changed the title wrap QMatrix in namespace pcl to resolve linker conflict wrap QMatrix in namespace pcl to resolve linker conflict with Qt6 Dec 10, 2021
@jspricke
Copy link
Copy Markdown
Member

@kunaltyagi Why is this flagged ABI break?
As far I can see this is an internal class.
Note that we can't have ABI breaks in patch releases as they don't bump the Soname, so either this should not have been in 1.12.1 or this is not an ABI break?

@kunaltyagi
Copy link
Copy Markdown
Member

kunaltyagi commented Dec 24, 2021

Yeah, I also thought there would be no ABI break, however, the tooling used (.github/generate_abi_reports.sh) verified the following:

  • change of namespace affected symbol names and signatures internal to the library, but the symbols are visible outside as well

This would not have resulted in ill-formed programs post linkage.

since this is a cpp file, the chances of someone actually relying on this are minimal and would require the person to be personally compiling PCL. Hence the decision to flag as ABI break for those people, but still send it in a minor release

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Dec 25, 2021

Here is the full report for the ml module:
compat_report_ml_1.12.1.zip

mvieth pushed a commit to mvieth/pcl that referenced this pull request Dec 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: ABI break Meta-information for changelog generation changelog: fix Meta-information for changelog generation module: ml

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[pcl_ml] both pcl_ml and QtGui defines QMatrix

5 participants