Skip to content

Commit

Permalink
Tools: Give ardupilot venv access to system packages
Browse files Browse the repository at this point in the history
* When possible, we can use the apt-installed python packages which are
  ABI stable
* Same for the other OS's that have VENV setup scripts

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
  • Loading branch information
Ryanf55 authored and peterbarker committed Sep 10, 2024
1 parent d4a62f2 commit d18a2b2
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion Tools/environment_install/install-prereqs-arch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ sudo usermod -a -G uucp "$USER"

sudo pacman -Syu --noconfirm --needed $BASE_PKGS $SITL_PKGS $PX4_PKGS

python3 -m venv "$HOME"/venv-ardupilot
python3 -m venv --system-site-packages "$HOME"/venv-ardupilot

# activate it:
SOURCE_LINE="source $HOME/venv-ardupilot/bin/activate"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ sudo usermod -a -G dialout "$USER"

$ZYPPER $BASE_PKGS $SITL_PKGS || echo "Check zypper output for errors"

python3 -m venv "$HOME"/venv-ardupilot
python3 -m venv --system-site-packages "$HOME"/venv-ardupilot

SHELL_LOGIN=".profile"
# activate it:
Expand Down
2 changes: 1 addition & 1 deletion Tools/environment_install/install-prereqs-ubuntu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ fi

if [ -n "$PYTHON_VENV_PACKAGE" ]; then
$APT_GET install $PYTHON_VENV_PACKAGE
python3 -m venv $HOME/venv-ardupilot
python3 -m venv --system-site-packages $HOME/venv-ardupilot

# activate it:
SOURCE_LINE="source $HOME/venv-ardupilot/bin/activate"
Expand Down

4 comments on commit d18a2b2

@Jaaaky
Copy link
Contributor

@Jaaaky Jaaaky commented on d18a2b2 Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but this caused lots of trouble and failures on openSUSE-Tumbleweed as a rolling release distro.
This should be reverted for Tools/environment_install/install-prereqs-openSUSE-Tumbleweed.sh at least.

I think it should not be used for arch too.

@peterbarker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jaaaky sorry for the breakage - what's the nature of the breakage you're seeing?

@Ryanf55 was this change tested?

@Jaaaky perhaps raising an issue to track this would be a good idea. OTOH, if these patches have not been tested then we should probably simply revert them.

@Ryanf55
Copy link
Collaborator Author

@Ryanf55 Ryanf55 commented on d18a2b2 Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's revert the others until someone requests this flag. No, I don't have openSUSE so I didn't test.
It was requested in dev call to update all the other scripts. I only proposed changing ubuntu.

@Ryanf55
Copy link
Collaborator Author

@Ryanf55 Ryanf55 commented on d18a2b2 Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#28273

Here's the patch.

Please sign in to comment.