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

Fix check for setup.cfg in build_ament_python.sh.in #48

Merged
merged 2 commits into from
Feb 6, 2024
Merged

Conversation

traversaro
Copy link
Member

@traversaro traversaro commented Feb 6, 2024

Fix #46 by reverting 8be7ab8 . To actually fix the regression of RoboStack/ros-humble#41 we need a rebuild of affected packages.

@Tobias-Fischer
Copy link
Contributor

Does that affect both linux and osx?

@Tobias-Fischer Tobias-Fischer merged commit 19111ec into master Feb 6, 2024
@Tobias-Fischer Tobias-Fischer deleted the fix46 branch February 6, 2024 23:22
@traversaro
Copy link
Member Author

Does that affect both linux and osx?

I guess yes, but I did not directly checked.

@Tobias-Fischer
Copy link
Contributor

Unfortunately the state is worse than before now. Copying from Slack:
ament-copyright does not have a setup.cfg. The pip variant installs the binaries in the correct path (bin), whereas the setup.py variant does not. So pip should be used, but currently setup.py is used.

demo-nodes-py does have a setup.cfg. So pip is used. However, it does not respect the install_scripts instructions within. Coincidentally, the setup.py variant would install the binaries in the correct location

It is almost as if the if condition should be negated.

@traversaro
Copy link
Member Author

Thanks, I was looking into that as well.

@traversaro
Copy link
Member Author

It is almost as if the if condition should be negated.

I did exactly this in #49, and it seems to work (tested in ament-copyright and in demo-nodes-py).

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.

Bug? Should this two lines checking setup.cfg be swapped?
2 participants