Fix package dir location

[dirk-thomas]

This replaces #747.

[asmodehn]

I tested this by running :

• cmake and make install
• python setup.py install

on my package that was causing problem before :

• with the fixed source expecting the new behavior https://github.com/asmodehn/flask-ext-catkin/commit/41fd3f7077ec7d8cb4a54a89ed830a212c387a5d inside a virtualenv,
• and compared with the old behavior on https://github.com/asmodehn/flask-ext-catkin/commit/de04b20773460d86f7d19b3970eb5728c09e901b , using the current version of catkin.

Everything seems to behave as expected with this pull request 👍

I also agree that the root location check had a problem, but I didn't check your fix in details yet. Especially how it would behave on sub-sub-packages ( need to make sure it checks two levels of directories I think... )

Anyway I ll use this version from now on with my projects, and I ll let you know if I see any issues...

Thanks a lot for the fix !

[asmodehn]

OK so I think I found a few issues with that location fix.

Here is the output of catkin_make, with print added in the location check :

 locations = _get_locations(pkgs, package_dir)
    for pkgname, location in locations.items():
        print("checking package {0} in {1}".format(pkgname, location))
        if not '.' in pkgname:
            continue
        splits = pkgname.split('.')
        # hack: ignore write-combining setup.py files for msg and srv files
        if splits[1] in ['msg', 'srv']:
            continue
        # check every child has the same root folder as its parent
        root_name = splits[0]
        root_location = location
        for _ in range(len(splits) - 1):
            root_location = os.path.dirname(root_location)
        print("checking {0} != {1}".format(root_location, locations[root_name]))
        if root_location != locations[root_name]:
            raise RuntimeError(
                "catkin_export_python does not support setup.py files that combine across multiple directories: %s in %s, %s in %s" % (pkgname, location, root_name, locations[root_name]))

On that revision https://github.com/asmodehn/flask-ext-catkin/commit/59120f6246806bab25f5e4c2fea06bd847b28e52
I have manually edited this to show the ones that I think should do a different check :

checking package passlib.ext in passlib/passlib/ext
checking passlib/passlib != passlib/passlib
checking package webargs in webargs/webargs
checking package celery.contrib in celery/celery/contrib
checking celery/celery != celery/celery
checking package kombu.async in kombu/kombu/async
checking kombu/kombu != kombu/kombu
checking package flask_wtf.recaptcha in flask-wtf/flask_wtf/recaptcha
checking flask-wtf/flask_wtf != flask-wtf/flask_wtf
checking package billiard.dummy in billiard/billiard/dummy
checking billiard/billiard != billiard/billiard
checking package flask_cors in flask-cors/flask_cors
checking package celery.worker in celery/celery/worker
checking celery/celery != celery/celery
checking package celery.bin in celery/celery/bin
checking celery/celery != celery/celery
checking package tornado_cors in tornado-cors/tornado_cors
checking package flower.views in flower/flower/views
checking flower/flower != flower/flower
checking package click in click/click
checking package flask_sqlalchemy in flask-sqlalchemy/flask_sqlalchemy
checking package celery.events in celery/celery/events
checking celery/celery != celery/celery
checking package flower in flower/flower
checking package billiard.py2 in billiard/billiard/py2
checking billiard/billiard != billiard/billiard
checking package flask_script in flask-script/flask_script
checking package kombu.transport.sqlalchemy in kombu/kombu/transport/sqlalchemy
checking kombu/kombu != kombu/kombu              <= WRONG CHECK should be kombu/kombu/transport/
checking package passlib in passlib/passlib
checking package passlib._setup in passlib/passlib/_setup
checking passlib/passlib != passlib/passlib
checking package celery in celery/celery
checking package flask_wtf in flask-wtf/flask_wtf
checking package kombu in kombu/kombu
checking package celery.utils in celery/celery/utils
checking celery/celery != celery/celery
checking package celery.backends.database in celery/celery/backends/database
checking celery/celery != celery/celery        <= WRONG CHECK should be celery/celery/backends/
checking package billiard.py3 in billiard/billiard/py3
checking billiard/billiard != billiard/billiard
checking package flask_restful.representations in flask-restful/flask_restful/representations
checking flask-restful/flask_restful != flask-restful/flask_restful
checking package kombu.transport.virtual in kombu/kombu/transport/virtual
checking kombu/kombu != kombu/kombu    <= WRONG CHECK should be kombu/kombu/transport/
checking package celery.backends in celery/celery/backends
checking celery/celery != celery/celery
checking package celery.concurrency in celery/celery/concurrency
checking celery/celery != celery/celery
checking package flask_security in flask-security/flask_security
checking package celery.app in celery/celery/app
checking celery/celery != celery/celery
checking package flask_restful.utils in flask-restful/flask_restful/utils
checking flask-restful/flask_restful != flask-restful/flask_restful
checking package passlib.ext.django in passlib/passlib/ext/django
checking passlib/passlib != passlib/passlib  <= WRONG CHECK should be passlib/passlib/ext/
checking package celery.loaders in celery/celery/loaders
checking celery/celery != celery/celery
checking package flower.utils.backports in flower/flower/utils/backports
checking flower/flower != flower/flower       <= WRONG CHECK should be flower/flower/utils/
checking package celery.fixups in celery/celery/fixups
checking celery/celery != celery/celery
checking package celery.security in celery/celery/security
checking celery/celery != celery/celery
checking package celery.utils.dispatch in celery/celery/utils/dispatch
checking celery/celery != celery/celery     <= WRONG CHECK should be  celery/celery/utils/
checking package kombu.transport in kombu/kombu/transport
checking kombu/kombu != kombu/kombu
checking package flask_restful in flask-restful/flask_restful
checking package celery.apps in celery/celery/apps
checking celery/celery != celery/celery
checking package celery.task in celery/celery/task
checking celery/celery != celery/celery
checking package passlib.tests in passlib/passlib/tests
checking passlib/passlib != passlib/passlib
checking package kombu.utils in kombu/kombu/utils
checking kombu/kombu != kombu/kombu
checking package passlib.utils._blowfish in passlib/passlib/utils/_blowfish
checking passlib/passlib != passlib/passlib     <= WRONG CHECK should be passlib/passlib/utils/
checking package passlib.utils in passlib/passlib/utils
checking passlib/passlib != passlib/passlib
checking package passlib.handlers in passlib/passlib/handlers
checking passlib/passlib != passlib/passlib
checking package aniso8601 in aniso8601/aniso8601
checking package billiard in billiard/billiard
checking package flower.api in flower/flower/api
checking flower/flower != flower/flower
checking package flower.utils in flower/flower/utils
checking flower/flower != flower/flower
checking package flask_migrate in flask-migrate/flask_migrate

Please let me know if my expectation is correct and if you can confirm that is a problem.

[asmodehn]

Also please let me know if you can reproduce and confirm this behavior.
We might want to check that in unit tests too actually...

[asmodehn]

Interestingly I am experiencing another (related ? ) problem with that version :

If my generate_distutils_setup() call contains a py_modules parameter, all the python modules listed in it are not present in devel/.
But they are present in install/.

You might be able to reproduce it ( with this catkin version ) on this version : https://github.com/asmodehn/flask-ext-catkin/commit/e3ffc19a0c30ebc7327f5cc6a22efe776b96d77c

No time to investigate now though. I ll get back to it another time, but please let me know if you find something in the meantime.

[dirk-thomas]

Looking at the list of checks performed this is exactly what I expected. For each package only the most parent package location is checked. That's why the code does:

root_name = splits[0]

and

root_location = location
for _ in range(len(splits) - 1):
    root_location = os.path.dirname(root_location)

This ensures that a subpackage is also a subfolder (and not being located somewhere independently).

Regarding your second comment about py_modules: that is currently not supported as listed as such in the docs (https://github.com/ros/catkin/blob/indigo-devel/doc/user_guide/setup_dot_py.rst#user-content-develspace-limitations)

[asmodehn]

Okay thanks for the link I didnt know about this devel space limitations... i ll have to find a work around on my side then.