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

Add --only-root option to install command, excluding all dependencies. #5783

Merged
merged 10 commits into from
Jun 15, 2022

Conversation

chadac
Copy link
Contributor

@chadac chadac commented Jun 6, 2022

Per the related issue, added the --only-root option to the InstallCommand (as hinted in the issue) to exclude all dependency groups from install.

This is already expected behavior on the part of the installer so no additional tests are needed to ensure this works with an empty list of dependency groups.

Pull Request Check List

Resolves: #3377

  • Added tests for changed code.
  • Updated documentation for changed code.

@chadac chadac marked this pull request as ready for review June 6, 2022 15:37
docs/cli.md Show resolved Hide resolved
@radoering
Copy link
Member

Out of curiosity: What happens if I run poetry install --only-root --no-root?

@chadac
Copy link
Contributor Author

chadac commented Jun 6, 2022

Out of curiosity: What happens if I run poetry install --only-root --no-root?

Haha, good question :) I tried it out, looks like it's effectively a noop minus creating the lockfile if it isn't present.

@neersighted
Copy link
Member

neersighted commented Jun 6, 2022

Out of curiosity: What happens if I run poetry install --only-root --no-root?

Haha, good question :) I tried it out, looks like it's effectively a noop minus creating the lockfile if it isn't present.

I'd like to see a meaningful message for the user when this case happens, e.g.

if self.option("extras") and self.option("all-extras"):
self.line_error(
"<error>You cannot specify explicit"
" `<fg=yellow;options=bold>--extras</>` while installing"
" using `<fg=yellow;options=bold>--all-extras</>`.</error>"
)
return 1

@abn
Copy link
Member

abn commented Jun 6, 2022

Is this something that should apply to all group enabled commands? For example, this won't make much sense for update or export.

@chadac
Copy link
Contributor Author

chadac commented Jun 6, 2022

Out of curiosity: What happens if I run poetry install --only-root --no-root?

Haha, good question :) I tried it out, looks like it's effectively a noop minus creating the lockfile if it isn't present.

I'd like to see a meaningful message for the user when this case happens, e.g.

if self.option("extras") and self.option("all-extras"):
self.line_error(
"<error>You cannot specify explicit"
" `<fg=yellow;options=bold>--extras</>` while installing"
" using `<fg=yellow;options=bold>--all-extras</>`.</error>"
)
return 1

Good point, I've gone ahead and added logic to catch this similar to that case, and set it up to exit when both options are present.

Should I also have this exit when both --only-root and --with/--without/--only are also passed? Right now it's currently a warning.

Is this something that should apply to all group enabled commands? For example, this won't make much sense for update or export.

True, I've gone ahead and migrated it into install.py and modified the logic accordingly.

@neersighted
Copy link
Member

Should I also have this exit when both --only-root and --with/--without/--only are also passed? Right now it's currently a warning.

I personally prefer an error as well, because an inattentive developer may munge a command line out of history. If they don't notice a warning message they may never understand the unexpected/inconsistent results they get until much later. I'd rather fail first as what the user is asking us to do is nonsensical and we should ask them to re-evaluate their request.

(this is all hypothetical of course, I would never make a silly mistake like re-using a command from history and combining options in a nonsensical manner 😄)

@chadac
Copy link
Contributor Author

chadac commented Jun 6, 2022

I personally prefer an error as well, because an inattentive developer may munge a command line out of history. If they don't notice a warning message they may never understand the unexpected/inconsistent results they get until much later. I'd rather fail first as what the user is asking us to do is nonsensical and we should ask them to re-evaluate their request.

(this is all hypothetical of course, I would never make a silly mistake like re-using a command from history and combining options in a nonsensical manner smile)

Yeah, that's definitely true. Plus I imagine swapping warnings to errors in the future is a whole lot harder than the other way around... might need to keep that in mind for some of my own tools lol

@chadac chadac changed the title Add --only-root option to group commands, excluding all dependencies. Add --only-root option to install command, excluding all dependencies. Jun 9, 2022
docs/cli.md Outdated Show resolved Hide resolved
docs/managing-dependencies.md Outdated Show resolved Hide resolved
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Immitate pip install --no-deps / Add --only-root
4 participants