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

Postinstall scripts can fail silently when executed incorrectly #66

Closed
ncoghlan opened this issue Nov 4, 2024 · 1 comment · Fixed by #69
Closed

Postinstall scripts can fail silently when executed incorrectly #66

ncoghlan opened this issue Nov 4, 2024 · 1 comment · Fixed by #69
Assignees
Labels
Category: Bug Something isn't working

Comments

@ncoghlan
Copy link
Collaborator

ncoghlan commented Nov 4, 2024

While working on #24/#64, I found a problem where the postinstall scripts will do the wrong thing if executed with something other than the expected base runtime environment (pyvenv.cfg gets populated incorrectly, so the layered environment ends up referencing the wrong copy of the base runtime environment).

LM Studio itself does this right, so it didn't hit the problem. The test suite is doing it wrong, but wasn't failing due to the deployed environments and build environments both existing in the CI environment.

The new automatic layer versioning tests in #64 picked up the problem because the expected contents of sys.path now differ between the built environments and the deployed environments for test_sample_project, making the misconfigured environments fail their checks (on Windows - the symlinked environments on Linux and macOS still worked due to the way CPython behaves when run via a symlink, even without pyvenv.cfg being set up yet).

@ncoghlan
Copy link
Collaborator Author

ncoghlan commented Nov 4, 2024

Resolution will be a partial implementation of #19, switching to a static data-driven postinstall script that reads a JSON layer config file from ./share/venv/metadata/venvstacks_layer.json to determine what to include in pyvenv.cfg.

sitecustomize.py will also read the file to determine:

* what sys.path entries to add
* what os.add_dll_directory calls to make (if any)

(rather than dynamically generating the sitecustomize.py file at build time)

Reading the JSON file on every startup would have been pointlessly slow, so sitecustomize.py will still be generated, it will just be generated in the post-install script for the deployed environments.

The post-install script implementation will be importable at build time, so the build environment sitecustomize.py can be generated at the same time as the venvstacks_layer.json config metadata for the deployed environment.

ncoghlan added a commit that referenced this issue Nov 5, 2024
* Allow postinstall scripts to be executed with
  any Python interpreter (not just the deployed
  base runtime interpreter)
* Generate layer config file as part of layers
* Use a common postinstall script in all layers
* Generate the deployed `sitecustomize.py` file
  from the layer config in the postinstall script
* Add unit tests for the common postinstall script

Closes #66. Implements initial steps towards #19.
ncoghlan added a commit that referenced this issue Nov 7, 2024
The updates for #66 are substantially changing the way the
deployed environment checks work in the test suite.

To make that change easier to review, first deduplicate
the deployed environment checking code.
ncoghlan added a commit that referenced this issue Nov 7, 2024
The updates for #66 are substantially changing the way the
deployed environment checks work in the test suite.

To make that change easier to review, first deduplicate
the deployed environment checking code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant