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: os.rename -> shutil.move #534

Merged
merged 3 commits into from
Apr 3, 2024
Merged

fix: os.rename -> shutil.move #534

merged 3 commits into from
Apr 3, 2024

Conversation

westonsteimel
Copy link
Contributor

@westonsteimel westonsteimel commented Apr 3, 2024

Replace instances of os.rename with shutil.move to handle the possibility of the destination being a different filesystem. If the source and destination are on the same filesystem, os.rename is used per https://docs.python.org/3.11/library/shutil.html#shutil.move

@westonsteimel westonsteimel changed the title fix: use shutil.move in place of os.rename when moving from temporary… fix: use shutil.move in place of os.rename when moving from temporary file Apr 3, 2024
@westonsteimel westonsteimel changed the title fix: use shutil.move in place of os.rename when moving from temporary file fix: os.rename -> shutil.move for temp files Apr 3, 2024
@westonsteimel westonsteimel added the run-pr-quality-gate Triggers running of quality gate on PRs label Apr 3, 2024
Replace instances of `os.rename` with `shutil.move` to handle the
possibility of the destination being a different filesystem.  If the
source and destination are on the same filesystem, `os.rename` is
used per https://docs.python.org/3.11/library/shutil.html#shutil.move

Signed-off-by: Weston Steimel <commits@weston.slmail.me>
@westonsteimel westonsteimel changed the title fix: os.rename -> shutil.move for temp files fix: os.rename -> shutil.move Apr 3, 2024
As opposed to rename, when moving a dir to a dir, shutil.move will try
to nest src under dst.

Signed-off-by: Will Murphy <will.murphy@anchore.com>
@@ -243,17 +244,17 @@ def overlay_existing(self, source: str, move: bool = False) -> None:
os.makedirs(os.path.dirname(dst), exist_ok=True)

if move:
os.rename(src, dst)
shutil.move(src, dst)
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, we might need the same change here, and it's just not failing a test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is overlay_existing actually used anywhere? I can't find any invocations of it. Maybe we can just remove it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see it used anywhere. I think it was added by mistake in #520, since that PR went through a lot of iterations.

I've pushed a commit removing the function.

Signed-off-by: Will Murphy <will.murphy@anchore.com>
@westonsteimel westonsteimel marked this pull request as ready for review April 3, 2024 14:38
@westonsteimel westonsteimel requested a review from a team April 3, 2024 14:39
@westonsteimel westonsteimel enabled auto-merge (squash) April 3, 2024 15:09
@westonsteimel westonsteimel merged commit 595a29b into main Apr 3, 2024
10 checks passed
@westonsteimel westonsteimel deleted the shutil-move branch April 3, 2024 15:17
@westonsteimel westonsteimel added the bug Something isn't working label Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working run-pr-quality-gate Triggers running of quality gate on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants