Skip to content
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

Virtual env runtime fix for running as Linux AppImage. #1863

Merged
merged 6 commits into from
Mar 22, 2022

Conversation

tmontes
Copy link
Member

@tmontes tmontes commented Oct 23, 2021

FACT:

  • While exploring Linux AppImage packaging in this pup PR I found a serious limitation.
  • Each time Mu runs (as an AppImage thingie) it is mounted under a different mount point.
  • This invalidates existing virtual envs because their interpreter -- a symlink to the python executable in Mu's installation -- no longer exists (it does, but each time under a different path).

THIS PR:

  • Detects that we're runing as an AppImage.
  • Re-symlinks the existing virtual env (if any) interpreter to the currently valid system interpreter.

PS: Isn't packaging fun? :)

Comment on lines 707 to 708
os.unlink(self.interpreter)
os.symlink(sys.executable, self.interpreter)
Copy link
Member

Choose a reason for hiding this comment

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

We've seen so many weirdness with how the virtual environments copy, create, or symlink the executables that I wouldn't be surprised if this breaks somewhere.

I think either the venv or virtualenv modules might have some utility to do this kind of thing.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, so the venv module has an upgrade option:

--upgrade             Upgrade the environment directory to use this version
                        of Python, assuming Python has been upgraded in-place.

Not sure if this would fall within the "assuming Python has been upgraded in-place", maybe not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Generally agreed: the --upgrade venv option migh serve us, but we're using virtualenv which, from what I've seen, does not expose such a "feature".

Maybe for "minimal guarantees" we could use the --symlinks (which defaults to True) when calling python -m virtualenv in mu/virtual_environment.py:773. Other than that, not really sure... :)

@tmontes
Copy link
Member Author

tmontes commented Nov 1, 2021

Updated the PR:

  • Use the --symlink option when creating the virtual env on non-Windows platforms.
    (wondering about the filter approach I took: can it be made more readable?)
  • Ignore (but log) failures in removing what is expected to be a stale symlink.

@tjguk, @carlosperate, @ntoll: your input is welcome. :)

@carlosperate
Copy link
Member

I'm very low on bandwidth to be able to test this until the end of the month, but the code looks good to me and I'd be happy for the PR to be merged it if you've already tested it 👍

@carlosperate carlosperate modified the milestones: 1.1.0-rc.1, 1.2 Dec 17, 2021
@carlosperate
Copy link
Member

I'd suggest to wait for the v1.1 final release before we merge AppImage support, we can then start testing this on Linux on with the first 1.2-beta.

@ntoll
Copy link
Member

ntoll commented Dec 20, 2021

Ack on post 1.1.

@tmontes
Copy link
Member Author

tmontes commented Mar 22, 2022

NOTE:

  • Python 3.7 on macOS had failed for a non-obvious motive: retriggered the job run.

@tmontes tmontes merged commit 324c576 into mu-editor:master Mar 22, 2022
@tmontes tmontes deleted the work-under-linux-appdir branch March 22, 2022 20:41
@carlosperate carlosperate modified the milestones: Future release, 1.2.0 Mar 24, 2022
@carlosperate carlosperate modified the milestones: 1.2.0, 1.1.2 May 20, 2022
"-p",
sys.executable,
"-q",
"" if self._is_windows else "--symlinks",
Copy link
Member

Choose a reason for hiding this comment

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

Turns out the --symlinks flag was only added to the virtualenv pacakge in version 20.0.0.
We currently support v16 and Debian/Raspbian Buster ships with v15, so in those environments this fails the virtualenv creation.

Copy link
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants