Project

General

Profile

Actions

Bug #22822

closed

Path rewrite of `activate` script for Python packages isn't working

Added by Brett Smith 11 months ago. Updated 11 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Deployment
Target version:
Story points:
-
Release relationship:
Auto

Description

shell:~ % export PATH=/usr/bin
shell:~ % . /usr/lib/python3-arvados-python-client/bin/activate
(python3-arvados-python-client) shell:~ % echo $PATH
/tmp/pybuild.EreXEM7D/python3-arvados-python-client/bin:/usr/bin
# Suffice to say that /tmp directory doesn't exist
(python3-arvados-python-client) shell:~ % command -v arv-get
/usr/bin/arv-get
# This will work but you would expect it to be the copy in the virtualenv

We have code in source:build/run-library.sh that's meant to patch this up:

   elif [[ "$binfile" =~ /activate(.csh|.fish|)$ ]]; then
      sed -ri "s@VIRTUAL_ENV(=| )\".*\"@VIRTUAL_ENV\\1\"$sys_venv_dir\"@" "$binfile" 

But apparently there's a bug in there somewhere, maybe because it assumes the value is quoted:

shell:~ % grep VIRTUAL_ENV= /usr/lib/python3-arvados-python-client/bin/activate
VIRTUAL_ENV=/tmp/pybuild.EreXEM7D/python3-arvados-python-client

We could add a test to the package test scripts to activate the virtualenv and check that command -v python3 then points to the expected /usr/lib/PKGNAME/bin.


Subtasks 1 (0 open1 closed)

Task #22841: Review 22822-venv-pathResolvedPeter Amstutz04/30/2025Actions
Actions #1

Updated by Brett Smith 11 months ago

On reflection, I think we should fix this by just having the package build scripts set up the virtualenvs at the location where we actually want to install them (i.e., /usr/lib/PKGNAME), instead of using a temporary directory. They run in an ephemeral Docker container, so I don't see any reason not to do this. And then we can take all this fragile path-rewriting logic out of our build scripts, which will be a reliability win.

We can still add the activate test as part of this, that's a good idea anyway.

Actions #2

Updated by Brett Smith 11 months ago

  • Target version set to Development 2025-05-14
  • Assigned To set to Brett Smith
  • Status changed from New to In Progress
Actions #3

Updated by Brett Smith 11 months ago

22822-venv-path @ 429a8d59bb5bf7ec14d00f3a2f8332270719b88d

  • All agreed upon points are implemented / addressed.
    • Implements the strategy and test proposed above.
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • N/A
  • Code is tested and passing, both automated and manual, what manual testing was done is described
    • Tested with:
      WORKSPACE="$PWD" build/run-build-test-packages-one-target.sh --arch amd64 --target debian12 --force-build --only-build python3-arvados-python-client
  • Documentation has been updated.
    • N/A - this fixes bugs so the package supports features already documented
  • Behaves appropriately at the intended scale (describe intended scale).
    • No change in scale
  • Considered backwards and forwards compatibility issues between client and server.
    • N/A
  • Follows our coding standards and GUI style guidelines.
    • N/A (no shell style guide)
Actions #4

Updated by Brett Smith 11 months ago

  • Subtask #22841 added
Actions #5

Updated by Peter Amstutz 11 months ago

LGTM, sometimes the simplest solution is the best.

Actions #6

Updated by Brett Smith 11 months ago

  • Status changed from In Progress to Resolved
Actions #7

Updated by Brett Smith 11 months ago

  • Release set to 78
Actions

Also available in: Atom PDF