-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Setup: Add venv for Ubuntu Noble (24.04 LTS) #6235
base: master
Are you sure you want to change the base?
Conversation
cfc529f
to
2b409d8
Compare
Sphinxsetup.sh
Outdated
# Install flake8 | ||
python3 -m pip install --user --upgrade flake8==3.7.9 | ||
python3 -m pip install $PIP_USER_ARGUMENT --upgrade sphinx==${SPHINX_VERSION} "docutils<0.19" "requests>=2.31.0" sphinx-tabs==3.* lxml git+https://github.com/ArduPilot/sphinx_rtd_theme.git git+https://github.com/ArduPilot/sphinxcontrib-youtube.git beautifulsoup4 flake8==3.7.9 |
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.
Please leave these as separate lines.
The efficiency is in no way important for this script, and it's easier to tell what's gone wrong with the separate lines.
It's also easier to tell exactly what's going on with the separate lines
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.
I put them together as pip has a much improved dependency resolver that can handle version conflicts when given all of the information. I can separate them out. I'm going to update #4642 next to move the list of packages to something common between setup scripts.
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.
I'm not sure I follow. With the current --upgrade option doing this on separate lines could result in different behavior as follow on pip install commands do not take into account the previously commanded pinning. Only when given all packages at once can the dependency resolver correctly do both updating and pinning.
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.
Wait, pip will remove pins if you pass --upgrade
and are specifying a specific package?
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.
I would say that isn't a bad idea to regroup things together
Sphinxsetup.sh
Outdated
@@ -10,14 +10,24 @@ fi | |||
DISTRIBUTION_ID=$(lsb_release -i -s) | |||
if [ ${DISTRIBUTION_ID} = 'Ubuntu' ]; then | |||
DISTRIBUTION_CODENAME=$(lsb_release -c -s) | |||
if [ ${DISTRIBUTION_CODENAME} = 'focal' ] || [ ${DISTRIBUTION_CODENAME} = 'bionic' ]; then | |||
sudo add-apt-repository universe | |||
if [ ${DISTRIBUTION_CODENAME} = 'focal' ] || [ ${DISTRIBUTION_CODENAME} = 'bionic' ] || [ ${DISTRIBUTION_CODENAME} = 'noble' ]; then |
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.
Do we need universe for noble?
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.
I scanned my installed packages with aptitude search '?section (multiverse) ?installed'
and only got steam and lutris things back. Removing for nobel.
@peterbarker @khancyr awaiting approvals |
... want to know the impact of |
Pining only works for the current cmdline, so any further pip install -U will remove the pining to satisfy the dependency. |
OK, thanks for checking. I guess I have to withdraw my objections to the one-line thing, 'though it makes it a PITA to work out what the problems are. Could we at least use |
3d74a67
to
6357b52
Compare
6357b52
to
f60d96d
Compare
This should be good as is. I think a better way would be to check if the year of the Ubuntu release is greater than 24. That would keep it working on the non-LTS releases too. |
This is an update to #5481 for LTS Noble.
Same ideas and problems being solved here.