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

Do not use atomic move to move jdks #29

Merged
merged 2 commits into from
May 12, 2022
Merged

Conversation

CRogers
Copy link
Contributor

@CRogers CRogers commented May 12, 2022

Before this PR

Tried this out on some very large projects internally. Kept getting this kinds of errors:

Caused by: java.lang.RuntimeException: Could not move java home
Caused by: java.nio.file.FileSystemException: /Users/callumr/.gradle/caches/gradle-jdks/azul-zulu-15.40.19-15.0.7-b7ed6f7c59131899.in-progress-c5b252e4/zulu15.40.19-ca-jdk15.0.7-macosx_aarch64/zulu-15.jdk/Contents/Home -> /Users/callumr/.gradle/caches/gradle-jdks/azul-zulu-15.40.19-15.0.7-b7ed6f7c59131899: Directory not empty
        at com.palantir.gradle.jdks.JdkManager.jdk(JdkManager.java:81)
        at com.palantir.gradle.jdks.JdksPlugin.lambda$javaInstallationForLanguageVersion$5(JdksPlugin.java:86)

It seems when you try to move a directory using ATOMIC_MOVE and the target directory exists, it fails with a FileSystemException with a message Directory not empty. Not a FileAlreadyExistsException or DirectoryNotEmptyException. At least on macos.

Additionally, the docs for ATOMIC_MOVE are a little scary:

If the target file exists then it is implementation specific if the existing file is replaced or this method fails by throwing an IOException.

😨 😨 😨

After this PR

==COMMIT_MSG==
Do not atomically move the java home directory to avoid Could not move java home: Directory not empty exceptions
==COMMIT_MSG==

Possible downsides?

@CRogers CRogers requested a review from carterkozak May 12, 2022 17:15
@changelog-app
Copy link

changelog-app bot commented May 12, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Do not atomically move the java home directory to avoid Could not move java home: Directory not empty exceptions

Check the box to generate changelog(s)

  • Generate changelog entry

@CRogers CRogers merged commit 42668f0 into develop May 12, 2022
@svc-autorelease
Copy link
Collaborator

Released 0.7.0

@CRogers CRogers deleted the callumr/atomic-move-bad branch May 12, 2022 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants