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

Default --interactive to true across the commands #35165

Closed
baronfel opened this issue Sep 5, 2023 · 4 comments · Fixed by #47226
Closed

Default --interactive to true across the commands #35165

baronfel opened this issue Sep 5, 2023 · 4 comments · Fixed by #47226

Comments

@baronfel
Copy link
Member

baronfel commented Sep 5, 2023

Describe the bug

Many commands that interact with NuGet have an --interactive flag that is used to signal that the user can be prompted for additional credentials. We should default this to 'true' instead of false so that user-driven sessions Just Work ™️.

We have several different --interactive flags that we should change at the same time, and we should have detection of common ways to signal that the session is not interactive:

  • if stdin/stdout/stderr are redirected
  • if we are not running in an interactive shell (in Unix this is commonly if the prompt environment variable PS1 is set)
  • ???
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-NetSDK untriaged Request triage from a team member labels Sep 5, 2023
@MiYanni
Copy link
Member

MiYanni commented Sep 15, 2023

@baronfel
Some clarification/questions:

  • Default all --interactive flags to true
  • Automatically set --interactive to false when:
    • Input/output redirection
    • The shell/console is in an interactive mode
      • I did some searching, and found that there is another way to check this for bash: https://unix.stackexchange.com/a/26782/238217. Let me know if this is more idiomatic.
      • Does Windows have a way to determine this for command prompt? I believe PowerShell does... I'll do some research either way.
    • [new] Environment variables indicating being within CI
      • Many CI systems have environment variables set for this. We have code for this check already in our codebase.
    • [idea] If dotnet is being ran as a child process? Or something like that. Basically, for scenarios where we can determine something else (another application) is calling it.

@baronfel
Copy link
Member Author

minor typo on my part - we should default --interactive to false when the shell is not in an interactive mode. I'll fix that in the issue description.

That question is a pretty big one, and as you've seen there are a bunch of shell-specific ways to handle this. I have two concerns with this: a) we don't have a strong set of code patterns already for 'here is how to answer question X in shell Y' + 'here are the Z shells we support', and b) many of these shell-specific detections involve running shell builtin commands, which would be process overhead - this is the kind of thing I'd like to keep to a minimum to prevent unnecessary work. I do think having that framework for answering shell-specific questions in a per-unique-shell way would be really useful to have though!

I also agree with the CI check, good thinking. The child process idea is a little more nebulous to me - technically pushes glasses up nose we're always going to be the child process of the shell that spawned us - but more generally being a child process isn't a problematic thing here, the redirection of the stdin/stdout/stderr streams is what would actually prevent us from prompting for interactive states. It just so happens that it's quite common to be redirected when being spawned as a child process of some other command. Make sense?

@MiYanni
Copy link
Member

MiYanni commented Sep 15, 2023

@baronfel
Yup yup! That's why I asked. I was just trying to think of scenarios where there is no 'user' in the mix or ways we can determine that the one running the show is not human. I figured some of them would overlap depending on the scenario. So, the biggest thing is determining the 'interactive mode' scenario, which sounds like it'll take some prototyping/proof of concepts to really flesh out. I can get started on the other bits as they are fairly straightforward. Thanks!

@baronfel baronfel added this to the 10.0.1xx milestone Jan 30, 2025
@baronfel
Copy link
Member Author

We have some of this for Terminal Logger now, so we should be able to re-use that for determining the default for this flag in CI/non-CI scenarios.

@MiYanni MiYanni removed the untriaged Request triage from a team member label Feb 12, 2025
@MiYanni MiYanni removed their assignment Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants