-
Notifications
You must be signed in to change notification settings - Fork 145
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
Add consider_namespace_packages=False #766
Conversation
Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this! I left some things that should be fixed to get this in.
Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, I'll run CI on it next.
@clalancette success! |
@clalancette this should be backported right? I faced the issue on humble |
Do we really need it there? We know for sure Humble and Iron won't run into this. They are pinned to older versions of pytest. |
In the original issue @jtbandes mentioned
Can you point to where that pin is configured? |
My team is mostly running on humble and running into this issue constantly. We do pip install pytest. |
@clalancette I think I have a hint of what's happening, at least for my setup, but I'm assuming it's a similar issue for the others. In my dockerfile I have At first I assumed that this was expected because the master branch of colcon-core might be tracking the latest versions of pytest, however it reproduces even with I always hear that mixing @cottsay maybe you have an idea? |
The
The python team is doing quite a lot to try and drive this point home. |
🤦 I should have noticed that capital U when copying the command from here, I assumed that was the argument needed to pip install directly from a repo. Is it really necessary then or better to remove it from the documentation? However, even without the
|
In my case, the workflow is "just" running |
It isn't typically feasible to test all combinations of all recursive dependencies for a project. For colcon's CI, we test using the latest releases of all dependencies, so it makes sense to recommend the same practice for those installing using pip. Clearly there was a bug here that made it incompatible with newer pytest independent of colcon or its direct requirements. If you want a blessed set of known working version combinations, well that's what the system package manager is for and is one of many arguments to use that instead of pip.
I would guess that one of the pytest plugins that are getting installed requires a version newer than 6.2.5. While omitting If you don't have a really good reason to install colcon from pip, I recommend using your system's package manager to avoid these problems. |
Thanks a lot for the detailed explanation.
My "really good reason" was to use features on master not yet released but now 0.16.0 is released so I'll wait for the apt version. Anyway I don't want to hijack the discussion with my "personal setup problems", for others having this issue maybe try to find out at what point your pytest is getting upgraded involuntarily |
FWIW I just ran into this on Humble, but because I have a "hybrid" setup that has both a pure Python package and a ROS wrapper around it. I can live without it, but would appreciate a backport. |
@clalancette seems like multiple people would benefit from that backport even if it's not strictly required, can we have it if it has no downsides? |
* Add consider_namespace_packages=False Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com>
@clalancette I am getting the same error
in build_and_test CI job on Iron. |
* Add consider_namespace_packages=False Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com> (cherry picked from commit 07f4332)
We also run into this with Humble. Will this be backported? |
@Mergifyio backport humble iron |
✅ Backports have been created
|
* Add consider_namespace_packages=False Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com> (cherry picked from commit 07f4332)
* Add consider_namespace_packages=False Signed-off-by: Tony Najjar <tony.najjar.1997@gmail.com> (cherry picked from commit 07f4332)
Fixes #765. That's a quick patch to avoid an error but I don't know if it's the "cleanest" solution, please advise.
Needs backporting humble and iron