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

Avoid reinstall tool already installed #41746

Merged
merged 18 commits into from
Jun 27, 2024

Conversation

caiogranero
Copy link
Contributor

Fixes #40818

Context

Following the changes introduced in #37311, the dotnet tool install --global command began reinstalling tools that were already installed, which involved deleting and then adding the same tool version again. Before this release, it was not possible to install a version that was already installed.

This behavior change was reported in #40818. I propose a modification to avoid reinstalling tools that are already installed.

Change

The current installation flow involves uninstalling all matching tools and then installing them again. My change modifies this flow by first checking if the requested best match package version is already installed. If it is, the flow is stopped, and a message is printed: 'Tool '{0}' is already installed'.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Tools untriaged Request triage from a team member labels Jun 22, 2024
@caiogranero
Copy link
Contributor Author

@caiogranero please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

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

I think this looks good to me—thanks! The only thing I'm wondering at the moment is what its behavior is if you have a broken tool package; is there a way to repair it?

@caiogranero
Copy link
Contributor Author

I think this looks good to me—thanks! The only thing I'm wondering at the moment is what its behavior is if you have a broken tool package; is there a way to repair it?

Probably only uninstalling and installing again. Is there any way to know if a package is broken in execution time?

@Forgind
Copy link
Member

Forgind commented Jun 24, 2024

I think this looks good to me—thanks! The only thing I'm wondering at the moment is what its behavior is if you have a broken tool package; is there a way to repair it?

Probably only uninstalling and installing again. Is there any way to know if a package is broken in execution time?

It probably depends on the tool. My PM suggested --force; can you make sure that if you try to install the version you already have and specify --force, that it still does the install?

@caiogranero
Copy link
Contributor Author

caiogranero commented Jun 24, 2024

I think this looks good to me—thanks! The only thing I'm wondering at the moment is what its behavior is if you have a broken tool package; is there a way to repair it?

Probably only uninstalling and installing again. Is there any way to know if a package is broken in execution time?

It probably depends on the tool. My PM suggested --force; can you make sure that if you try to install the version you already have and specify --force, that it still does the install?

I can be wrong but I think dotnet tools doesn't support --force at this moment.

I couldn't find --force in dotnet tool docs and I also founded the issue #33246 regarding the same topic.

@Forgind
Copy link
Member

Forgind commented Jun 25, 2024

Since we're tracking --force, I think I'm happy with this. @nagilson, do you want to review before we merge?

Copy link
Member

@nagilson nagilson left a comment

Choose a reason for hiding this comment

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

The code looks great to me, you did a great job adding tests and adapting to the existing code base. Thank you for your contribution! :)

@nagilson
Copy link
Member

btw, the pipeline failures here are not your fault, it is this annoying issue that has been going on, we will try to take care of it.

@nagilson
Copy link
Member

nagilson commented Jun 26, 2024

/azp run

Copy link

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@caiogranero
Copy link
Contributor Author

The code looks great to me, you did a great job adding tests and adapting to the existing code base. Thank you for your contribution! :)

Thank you! I liked to contribute and you will see me more times here 😊

@caiogranero
Copy link
Contributor Author

@nagilson @Forgind All pipelines passed, could you merge?

@Forgind Forgind merged commit 4a4823d into dotnet:main Jun 27, 2024
36 checks passed
@Forgind
Copy link
Member

Forgind commented Jun 27, 2024

@caiogranero
Merged! Thanks so much for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Tools untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dotnet tool install --global reinstalls even if the same version is already present
3 participants