Install flake8 from proposed #761
Conversation
clalancette
force-pushed
the
claraberendsen/install-from-proposed
branch
2 times, most recently
from
9748432 to
370adea
Compare
There was a problem hiding this comment.
There appear to be at least three distinct principal changes stacked into this PR and a lot of other smaller commits that are quite difficult to follow.
@clalancette / @claraberendsen would you consider splitting this into
- The namedtuple refactor of pip dependencies.
- The refactor of --break-system-packages to use the environment variable
- The actual changes to install python3-flake8 from noble-proposed
And anything that doesn't fit in those to separate cleanup PRs. Or at least reasonably explained cleanup commits within those PRs.
This is one repository where history spelunking is often necessary when making changes and I would have a very hard time revisiting this PR and re-constructing the rationale behind all changes from the commit log or PR description as they are right now.
For CI, I think it would be alright to continue using this branch as an omnibus for tests rather than requiring each PR go through CI alone.
linux_docker_resources/Dockerfile
Outdated
| RUN if test \( ${UBUNTU_DISTRO} = noble \); then\ | ||
| echo "deb http://archive.ubuntu.com/ubuntu/ "$(lsb_release -cs)"-proposed restricted main multiverse universe" >> /etc/apt/sources.list.d/ubuntu-$(lsb_release -cs)-proposed.list \ | ||
| ;fi | ||
| # Install from proposed conflicting packages |
There was a problem hiding this comment.
What does conflicting mean in this context?
There was a problem hiding this comment.
It meant to mean packages that are broken/outdated on the repository and it needs to install from proposed instead.
But I can see why conflicting can be confusing, does something like necessary or broken better?
ros2_batch_job/__main__.py
Outdated
|
|
||
| pip_cmd = ['"%s"' % job.python, '-m', 'pip', 'install', '-c', 'constraints.txt', '-U'] | ||
| if sys.platform == 'win32': | ||
| # Force reinstall so all dependencies are in virtual environment | ||
| # On Windows since we switch between the debug and non-debug | ||
| # interpreter all packages need to be reinstalled too | ||
| pip_cmd.append('--force-reinstall') | ||
| else: | ||
| # We need to use --user so that certain packages (like flake8-blind-except) | ||
| # can successfully build on RHEL-8 (Humble). |
There was a problem hiding this comment.
In that case does it make sense to restrict the addition of this flag to the platform that requires it?
There was a problem hiding this comment.
In that case does it make sense to restrict the addition of this flag to the platform that requires it?
Yeah, but unfortunately here in this script, we don't have that information. I'm currently using humble as a proxy for it, but that is probably a bad idea overall. I'll think about how to do this better, we may have to add a new --platform flag.
| 'Press \[Enter\] to continue:', | ||
| 'Do you accept this license\? \[y/n\]: ']) | ||
| r'Press \[Enter\] to continue:', | ||
| r'Do you accept this license\? \[y/n\]: ']) |
There was a problem hiding this comment.
It's unclear from any commit message why this change was made alongside the break system packages environment variable usage.
|
@nuclearsandwich I agree on splitting the flake 8 PR, and link it back to this for testing. I think that overall issues are sistematic and getting the solution for noble given the time constraint we have would be easier to achieve if we have a unified testing branch that imports the changes, so we know when we merge all related PRs then noble should work . |
clalancette
force-pushed
the
claraberendsen/install-from-proposed
branch
from
6161f80 to
2429dca
Compare
|
The split PR for installing from proposed: #765 |
clalancette
force-pushed
the
claraberendsen/install-from-proposed
branch
2 times, most recently
from
c01210a to
c4a2025
Compare
|
FYI, I'm still using this PR as a way to test out various changes that we need for Noble. At this point I think what is in here actually makes Noble build; I still need to split out the various fixes into PRs. But anyway, here is a Noble build to see where we are:
|
clalancette
force-pushed
the
claraberendsen/install-from-proposed
branch
from
db6b37d to
2895116
Compare
|
Rebased onto the latest, and more fixes for further changes in Noble, here's another try:
|
clalancette
force-pushed
the
claraberendsen/install-from-proposed
branch
from
2895116 to
8f62eee
Compare
Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
clalancette
force-pushed
the
claraberendsen/install-from-proposed
branch
from
b422694 to
7eec874
Compare
|
Over the weekend, it looks like the rest of the packages we needed were rebuilt. I still believe that there is at least one more fix we need here for Humble, but that will be done in separate PRs. I'm going to close this one out for now, thanks @claraberendsen ! |
This is still a draft because things aren't totally working here; will need another update.