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

✨ install with pip [former] #35

Closed
wants to merge 12 commits into from
Closed

Conversation

juftin
Copy link
Owner

@juftin juftin commented Dec 6, 2023

Changes

  • New installer class
    • Changes default behavior to run pip install -r lockfile.txt instead of pip-sync lockfile.txt
      • Makes this a configurable option, pip-compile-installer
    • Only forces project reinstall when using pip-sync option
    • Deletes lockfile if no dependencies with both pip and pip-sync options

Yet another alternative to #30

Closes #29

Comment on lines +109 to +115
if not self.lockfile_up_to_date:
self.install_pip_tools()
if self.piptools_lock_file.exists():
_ = self.piptools_lock.compare_python_versions(
verbose=self.config.get("pip-compile-verbose", None)
)
self.pip_compile_cli()
Copy link
Owner Author

@juftin juftin Dec 6, 2023

Choose a reason for hiding this comment

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

There is a safe_activation context needed here

Suggested change
if not self.lockfile_up_to_date:
self.install_pip_tools()
if self.piptools_lock_file.exists():
_ = self.piptools_lock.compare_python_versions(
verbose=self.config.get("pip-compile-verbose", None)
)
self.pip_compile_cli()
if not self.lockfile_up_to_date:
with self.safe_activation():
self.install_pip_tools()
if self.piptools_lock_file.exists():
_ = self.piptools_lock.compare_python_versions(
verbose=self.config.get("pip-compile-verbose", None)
)
self.pip_compile_cli()

Comment on lines +119 to +120
with self.environment.safe_activation():
self.install_dependencies()
Copy link
Owner Author

Choose a reason for hiding this comment

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

See #34 (comment)

There is a scenario where the lockfile doesn't exist and install_dev_mode invokes _full_install without run_pip_compile ever being called

Suggested change
with self.environment.safe_activation():
self.install_dependencies()
with self.environment.safe_activation():
self.environment.run_pip_compile()
self.install_dependencies()

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah just commit these! Oh I see, you can't 😅
Maybe needs yet another new PR as per the other comment. Or let me know, I can just commit this

@juftin juftin self-assigned this Dec 6, 2023
@juftin juftin added the enhancement New feature or request label Dec 6, 2023
@juftin
Copy link
Owner Author

juftin commented Dec 6, 2023

I would expect an additional mention of you as sub-author here in the GitHub UI in addition to the squash commit message. Maybe because I opened the PR?

image

@oprypin
Copy link
Contributor

oprypin commented Dec 6, 2023

It might be messing up because the commit is in my repo 🤔

Regarding the co-authored-by - you can always add that string manually
But at this point I'm wondering if it's messing up and will not credit you as the author, only the commiter.
That being the reason maybe it thinks co-authored is redundant

You should perhaps just push the same branch to your repo (+ a new PR)

git checkout -b branchname
git fetch origin 00f5b13015c1b36318f53377a8a603cc2fb9c5a9
git reset --hard 00f5b13015c1b36318f53377a8a603cc2fb9c5a9
git push origin branchname

@juftin juftin changed the title ✨ install with pip ✨ install with pip [former] Dec 6, 2023
@juftin
Copy link
Owner Author

juftin commented Dec 6, 2023

#36 now 🤷

@juftin juftin closed this Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ choose between pip-sync and pip install -r
2 participants