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.

Sign up for GitHub

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

Jump to bottom

Defend against a missing WindowsRegistry service #29068

Merged
merged 2 commits into from May 9, 2024
Merged

Defend against a missing WindowsRegistry service #29068

merged 2 commits into from May 9, 2024

Conversation

big-guy
Copy link
Member

@big-guy big-guy commented May 8, 2024

fixes #29024

Contributor Checklist

  • Review Contribution Guidelines.
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
  • Provide unit tests (under <subproject>/src/test) to verify logic.
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes.
  • Ensure that tests pass sanity check: ./gradlew sanityCheck.
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@big-guy
Add more diagnostics to native services error message
591afee 
enabled is true when we think native-platform should be used.
os is the currently detected OS.
@big-guy big-guy changed the base branch from master to release May 8, 2024 21:14
@big-guy big-guy self-assigned this May 8, 2024
@big-guy big-guy added this to the 8.8 RC2 milestone May 8, 2024
@big-guy big-guy marked this pull request as ready for review May 8, 2024 21:15
@big-guy big-guy requested review from a team as code owners May 8, 2024 21:15
@big-guy big-guy requested a review from ghale May 8, 2024 21:15
@big-guy
Defensively protect against a broken WindowsRegistry service
1749f57 
Native-platform is sometimes unavailable.  Without native-platform, we're
unable to look-up things from the Windows registry.
@big-guy big-guy force-pushed the sg/88/regression/windowsregistry branch from a8931dc to 1749f57 Compare May 8, 2024 22:14
@big-guy
Copy link
Member Author

big-guy commented May 8, 2024

@bot-gradle test this

@bot-gradle
Copy link
Collaborator

I've triggered the following builds for you. Click here to see all build failures.

@gitstream-cm gitStream.cm
Copy link

gitstream-cm bot commented May 8, 2024

🥷 Code experts: lptr, adammurdoch

lptr, big-guy have most 👩‍💻 activity in the files.
lptr, adammurdoch have most 🧠 knowledge in the files.

See details

platforms/core-runtime/native/src/main/java/org/gradle/internal/nativeintegration/services/NativeServices.java

Activity based on git-commit:

lptr big-guy
MAY
APR
MAR
FEB 44 additions & 2 deletions
JAN
DEC 9 additions & 1 deletions

Knowledge based on git-blame:
adammurdoch: 36%
lptr: 11%

platforms/core-runtime/native/src/test/groovy/org/gradle/internal/nativeintegration/services/NativeServicesTest.groovy

Activity based on git-commit:

lptr big-guy
MAY
APR
MAR
FEB
JAN
DEC

Knowledge based on git-blame:
adammurdoch: 73%
lptr: 2%

platforms/jvm/jvm-services/build.gradle.kts

Activity based on git-commit:

lptr big-guy
MAY
APR
MAR 1 additions & 1 deletions
FEB
JAN 23 additions & 0 deletions
DEC

Knowledge based on git-blame:
lptr: 40%
adammurdoch: 5%

platforms/jvm/jvm-services/src/main/java/org/gradle/jvm/toolchain/internal/WindowsInstallationSupplier.java

Activity based on git-commit:

lptr big-guy
MAY
APR 3 additions & 5 deletions
MAR
FEB
JAN
DEC

Knowledge based on git-blame:

platforms/jvm/jvm-services/src/test/groovy/org/gradle/jvm/toolchain/internal/WindowsInstallationSupplierTest.groovy

Activity based on git-commit:

lptr big-guy
MAY
APR 1 additions & 1 deletions
MAR
FEB
JAN
DEC

Knowledge based on git-blame:

To learn more about /:\ gitStream - Visit our Docs

ghale
ghale approved these changes May 9, 2024
View reviewed changes
Copy link
Member

@ghale ghale left a comment

Choose a reason for hiding this comment

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

LGTM

@big-guy
Copy link
Member Author

big-guy commented May 9, 2024

@bot-gradle merge

@gradle gradle deleted a comment from big-guy May 9, 2024
@bot-gradle bot-gradle added this pull request to the merge queue May 9, 2024
Merged via the queue into release with commit 8920d62 May 9, 2024
47 checks passed
@bot-gradle bot-gradle deleted the sg/88/regression/windowsregistry branch May 9, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ghale ghale ghale approved these changes

@octylFractal octylFractal octylFractal approved these changes

Assignees

@big-guy big-guy

Labels
build-script-change platform:core_runtime platform:jvm
Projects
None yet
Milestone
8.8 RC2
Development

Successfully merging this pull request may close these issues.

8.8-rc-1: Daemon Toolchain Support on Windows: WindowsRegistry is not supported on this operating system.
4 participants
@big-guy @bot-gradle @ghale @octylFractal