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

Eagerly update app requirements for DevCommand #1719

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rmartin16
Copy link
Member

@rmartin16 rmartin16 commented Apr 4, 2024

Changes

  • When requirements are re-installed in to the build for BuildCommand, the installation target directory is deleted prior to installing the app's requirements. Effectively, this means the transitive requirements are eagerly updated since the resolver will re-run and determine the latest version to install.
  • To align with the behavior of BuildCommand, this updates DevCommand to eagerly update the app's transitive requirements when installing its direct requirements.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@rmartin16 rmartin16 marked this pull request as draft April 4, 2024 16:42
@rmartin16
Copy link
Member Author

For some of the output formats, Briefcase defers installing the app's requirements to the build system.

  • Android Gradle
    • Running briefcase build android -r doesn't seem to actually force a re-installation of the requirements...
    • Instead, some comparison must be being made to determine if the step can be skipped....perhaps similar to how Docker layer caching works
    • The implication therefore being requirements can only be updated with direct version pinning or re-creating the build cache
    • Would it be possible to update the Gradle build to respect a flag to force the re-installation of requirements?
  • Linux Flatpak
    • Each build always re-installs all direct and transitive requirements matching the behavior of pip install in BuildCommand
  • Web Static
    • Gathering/building wheels in BuildCommand seems to re-run the resolver each time for all requirements...

@rmartin16 rmartin16 marked this pull request as ready for review April 4, 2024 17:11
@freakboy3742
Copy link
Member

  • Running briefcase build android -r doesn't seem to actually force a re-installation of the requirements...
  • Would it be possible to update the Gradle build to respect a flag to force the re-installation of requirements?

It forces re-writing the requirements.txt file; but that doesn't touch transitive requirements (unless we were to modify Briefcase to output the requirements resolution, rather than just the explicit requirements. That might pair well with a solution to #476.

I don't know if the Chaquopy gradle plugin can be forced to do an eager update. AFAIK, it's essentially using a internally vendored fork of pip, so it might be possible - @mhsmith would need to answer that one.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I can see the edge case you're describing, but I'm not 100% convinced whether it's desirable or not.

Consistency with briefcase run is definitely a compelling argument. However, I'm wary that briefcase dev is running in a virtual environment that might be shared with other tools. Eagerly updating a transitive dependency could easily have unintended consquences - especially if there's no way to opt out of that mode of update.

For my money, this is almost papering over limitations that would be addressed by a combination of #476 and #431.

Interestingly, one of the possible solutions for #431 is to make dev use an isolated virtual environment, rather than the one that is running briefcase. In that context, eager update would absolutely make sense (and be consistent with run), and would have the additional benefit that pip install leakage would no longer be an issue - if you don't put a requirement in the requires list, it won't be available.

@mhsmith - Do you have any thoughts here?

@mhsmith
Copy link
Member

mhsmith commented Apr 5, 2024

I don't know if the Chaquopy gradle plugin can be forced to do an eager update. AFAIK, it's essentially using a internally vendored fork of pip, so it might be possible - @mhsmith would need to answer that one.

This is actually happening at the level of the Chaquopy Gradle plugin – it only reruns pip if the requirements have changed. Any change at all to the requirements file should be sufficient, including a comment.

@rmartin16
Copy link
Member Author

rmartin16 commented Apr 5, 2024

It forces re-writing the requirements.txt file;

it only reruns pip if the requirements have changed.

This could offer a rationale for inserting a datetime or something similar in requirements.txt each time it's written so Gradle detects a change and re-installs the requirements.

Eagerly updating a transitive dependency could easily have unintended consquences - especially if there's no way to opt out of that mode of update.

hmm....using the same python environment for disparate projects seems likely to create issues whether we do eager updates or not. But there's no real pressing issue to merge something like this in when other solutions could more holistically address this situation.

So, I'll will take a look at those references and see if anything looks doable right now.

[edit] Captured Android issue with -r in #1721

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

Successfully merging this pull request may close these issues.

3 participants