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

fix(commands/common/_copy_package_resource): if destination exists and files are same, do not overwrite existing files #1528

Merged
merged 3 commits into from
Sep 3, 2024

Conversation

pratikpc
Copy link
Contributor

@pratikpc pratikpc commented Sep 1, 2024

  • I have added a news fragment under changelog.d/ (if the patch affects the end users)

Summary of changes

  1. The approach is simple.
  2. On Windows, can_symlink is going to fail which means according to the code, _copy_package_resource is going to be called
  3. In _copy_package_resource, if dest exists and dest and src are the same files, I simply check if according to cmp they are the same files based on the suggestions of @itsayellow
  4. Non Shallow cmp has been used for sanity reasons
  5. This should ideally cause no issues on Linux or macOS, although that has not been tested

Test plan

Tested by running

# command(s) to exercise these changes
python -m pipx upgrade --verbose commitizen

Current output

Previous output

pipx >(setup:1110): pipx version is 1.7.1
pipx >(setup:1111): Default python interpreter is 'python.exe'
pipx >(run_pipx_command:241): Virtual Environment locations are:
- commitizen : pipx\venvs\commitizen
pipx >(_upgrade_venv:154): Ignoring --python as not combined with --install
pipx >(run_subprocess:175): running <checking pip's availability>
pipx >(needs_upgrade:99): Time since last upgrade of shared libs, in seconds: 351. Upgrade will be run by pipx if greater than 2592000.
pipx >(run_subprocess:175): running <checking pip's availability>
pipx >(run_subprocess:175): running <checking pip's availability>
pipx >(upgrade:119): Upgrading shared libraries in pipx\shared
upgrading shared libraries...
pipx >(run_subprocess:175): running pipx\shared\Scripts\python.exe -m pip --no-input --disable-pip-version-check install --upgrade pip >= 23.1
pipx >(_parsed_package_to_package_or_url:139): cleaned package spec: commitizen
pipx >(upgrade_package:457): Upgrading commitizen
upgrading commitizen...
pipx >(run_subprocess:175): running pipx\venvs\commitizen\Scripts\python.exe -m pip --no-input install --upgrade commitizen
pipx >(run_subprocess:175): running <fetch_info_in_venv commands>
pipx >(get_venv_metadata_for_package:351): get_venv_metadata_for_package: 391ms
pipx >(_parsed_package_to_package_or_url:139): cleaned package spec: commitizen
pipx >(_copy_package_resource:111): ⚠️  Overwriting file pipx-exe\cz.exe with pipx\venvs\commitizen\Scripts\cz.exe
pipx >(_copy_package_resource:111): ⚠️  Overwriting file pipx-exe\git-cz.exe with pipx\venvs\commitizen\Scripts\git-cz.exe
commitizen is already at latest version 3.29.0 (location: pipx\venvs\commitizen)
pipx >(setup:1110): pipx version is 1.7.1
pipx >(setup:1111): Default python interpreter is 'python.exe'
pipx >(run_pipx_command:241): Virtual Environment locations are:
- commitizen : pipx\venvs\commitizen
pipx >(_upgrade_venv:154): Ignoring --python as not combined with --install
pipx >(run_subprocess:175): running <checking pip's availability>
pipx >(needs_upgrade:99): Time since last upgrade of shared libs, in seconds: 763. Upgrade will be run by pipx if greater than 2592000.
pipx >(run_subprocess:175): running <checking pip's availability>
pipx >(run_subprocess:175): running <checking pip's availability>
pipx >(upgrade:119): Upgrading shared libraries in pipx\shared
upgrading shared libraries...
pipx >(run_subprocess:175): running pipx\shared\Scripts\python.exe -m pip --no-input --disable-pip-version-check install --upgrade pip >= 23.1
pipx >(_parsed_package_to_package_or_url:139): cleaned package spec: commitizen
pipx >(upgrade_package:457): Upgrading commitizen
upgrading commitizen...
pipx >(run_subprocess:175): running pipx\venvs\commitizen\Scripts\python.exe -m pip --no-input install --upgrade commitizen
pipx >(run_subprocess:175): running <fetch_info_in_venv commands>
pipx >(get_venv_metadata_for_package:351): get_venv_metadata_for_package: 323ms
pipx >(_parsed_package_to_package_or_url:139): cleaned package spec: commitizen
commitizen is already at latest version 3.29.0 (location: pipx\venvs\commitizen)

Other Platforms

Due to an absence of a macOS device, I would be unable to test my patch on macOS. The same also applies to a large extent to Linux so assistance for the same would be welcome!!!

Fixes #683

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Sounds reasonable to me.

pratikpc and others added 3 commits September 3, 2024 19:51
…d files are same, do not overwrite existing files

This needs to be performed because on Windows, can_symlink traditionally fails. This means that every time there's an upgrade command run, the previous file is deleted and a new file is created which is a bit wasteful. This ensures that such issues do not happen.

Closes #683
@dukecat0 dukecat0 enabled auto-merge (squash) September 3, 2024 11:51
@dukecat0 dukecat0 merged commit 6dd82e0 into pypa:main Sep 3, 2024
11 checks passed
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.

pipx upgrade on Windows prints warnings about overwriting apps
3 participants