add sip5 binding install directory check #95
Conversation
|
@dirk-thomas can you take a quick look at this patch? |
|
@dirk-thomas @claireyywang @sloretz it has been more than 2 week since this is open. Can you provide closure to this PR? |
|
I am not maintaining this repository anymore. That is why I assigned the new maintainers to review this PR. |
|
@claireyywang @sloretz it has been around a month since this is open. Can you provide closure to this PR? |
|
@claireyywang @sloretz friendly ping |
1 similar comment
|
@claireyywang @sloretz friendly ping |
|
@acxz Sorry about the delay, I'll take a look soon! |
|
@claireyywang @sloretz it has been around 2 months since this PR has been opened, sorry to keep bugging you guys about it, but it really is a quick fix. It would be much appreciated if you guys can provide closure to this PR. |
|
@claireyywang @sloretz would be much appreciated if you can take a glance at this. |
|
@claireyywang @sloretz another ping. |
|
@claireyywang @sloretz it has been a couple months now if you guys could take just a quick look at this, it would be much much appreciated! |
|
@claireyywang @sloretz another ping. :( |
| @@ -61,6 +61,12 @@ def get_sip_dir_flags(config): | |||
| # sipconfig.Configuration does not have a pyqt_sip_dir or pyqt_sip_flags AttributeError | |||
| sip_flags = QtCore.PYQT_CONFIGURATION['sip_flags'] | |||
|
|
|||
| # sip5 installs here by default | |||
| default_sip_dir = os.path.join(sipconfig._pkg_config['default_mod_dir'], 'PyQt5', 'bindings') | |||
There was a problem hiding this comment.
I launched an archlinux container and confirmed this is the right location.
>>> sipconfig._pkg_config['default_mod_dir']
'/usr/lib/python3.9/site-packages'
>>> sipconfig._pkg_config['default_sip_dir']
'/usr/share/sip'It does seem strange since default_sip_dir seems like it should be the right spot.
default_sip_dir
The name of the base directory where the .sip files for SIP generated modules should be installed by default. A sub-directory with the same name as the module should be created and its .sip files should be installed in the sub-directory. The .sip files only need to be installed if you might want to build other bindings based on them.
Is it possible this is an archlinux specific packaging bug? Maybe introduced here? If so, I would be happy to merge this addition so long as the comment says it's a workaround for packaging on archlinux.
There was a problem hiding this comment.
thx for looking into this! Yeah I'll update the comment to be more reflective of the situation.
There was a problem hiding this comment.
Renamed the comment and commit to mention the change is due to archlinux specifics.
There was a problem hiding this comment.
There is nothing archlinux specific here. You are looking at the sip 4 install dir, this change is for sip >= 5 (latest version is 6.x)
|
thank you for considering my PR! |
|
@sloretz Would it be possible to get a ROS 1 release which includes this change? We can use it from the hash in the short term, but we need it for the environment we're building in. |
|
@mikepurvis released in ros/rosdistro#30232 and ros/rosdistro#30233 |
|
Awesome, thanks! |
resolves #80
affected downstream issues:
ros-noetic-arch/ros-noetic-python-qt-binding#2
ros-melodic-arch/ros-melodic-rviz#9
Basically adds the sip5 default binding install directory to the check, since this will be the new place of bindings and some distros have already switched causing errors on those systems without adding this check.
Added comments in code for clarity.