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

Set virtual project directories for :bom, :version-catalog #5664

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Set virtual project directories for :bom, :version-catalog #5664

wants to merge 2 commits into from
+12 −0

Conversation

sh-cho
Copy link
Contributor

@sh-cho sh-cho commented May 4, 2024 •
edited

Motivation:

Starting with Gradle 9.0, Gradle will not run builds if a project directory is missing or read-only.
(ref:
https://docs.gradle.org/8.7/userguide/upgrading_version_8.html#deprecated_missing_project_directory)

Modification:

Result:

  • Deprecation for Gradle 9.0 is prepared for this behavior

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 4, 2024 •
edited

🔍 Build Scan® (commit: 9dc8494)

Job name Status Build Scan®
build-windows-latest-jdk-21 ❌ (failure) https://ge.armeria.dev/s/fjto647dmtxg2
build-self-hosted-unsafe-jdk-8 ✅ https://ge.armeria.dev/s/2y5q2e3bq6stu
build-self-hosted-unsafe-jdk-21-snapshot-blockhound ❌ (failure) https://ge.armeria.dev/s/w7xlwn6qqhhd6
build-self-hosted-unsafe-jdk-17-min-java-17-coverage ✅ https://ge.armeria.dev/s/jq6sruqrmdo66
build-self-hosted-unsafe-jdk-17-min-java-11 ✅ https://ge.armeria.dev/s/bqa2oderenbbw
build-self-hosted-unsafe-jdk-17-leak ✅ https://ge.armeria.dev/s/rcze2yywpsva6
build-self-hosted-unsafe-jdk-11 ✅ https://ge.armeria.dev/s/xlmljfg7xhsw4
build-macos-12-jdk-21 ✅ https://ge.armeria.dev/s/acuzrnbjh2gm6

jrhee17
jrhee17 approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

Verified with the warning message with ./gradlew publishToMavenLocal --parallel --warning-mode all.

A warning message for dependencyManagement is also printed, but I prefer that it be handled when we actually upgrade the gradle version. Thanks @sh-cho 👍 👍 👍

@jrhee17 jrhee17 added this to the 1.29.0 milestone May 7, 2024
ikhoon
ikhoon reviewed May 7, 2024
View reviewed changes
settings.gradle Outdated
// Published version catalog projects
includeWithFlags ':version-catalog', 'version-catalog'
project(':version-catalog').projectDir.mkdirs()
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Should we create these projects under gradle/virtual-projects?
  • Add gradle/virtual-projects/ to .gitignore?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe gradle/scripts/.virtual-projects/? I wonder if it makes sense to use build/virtual-projects as well.. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

build/virtual-projects sounds good because build/ is already excluded from git and is automatically cleaned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I'll try to move generated projects under build/virtual-projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done :)
Please take a look

@sh-cho sh-cho force-pushed the gradle-missing-project-dir branch from d57fff3 to 12edadf Compare May 17, 2024 15:04
@sh-cho sh-cho changed the title Create project directories for :bom, :version-catalog Create virtual project directories for :bom, :version-catalog May 17, 2024
@sh-cho sh-cho changed the title Create virtual project directories for :bom, :version-catalog Set virtual project directories for :bom, :version-catalog May 17, 2024
@sh-cho sh-cho requested a review from jrhee17 May 17, 2024 15:42
minwoox
minwoox approved these changes May 20, 2024
View reviewed changes
Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Thanks a lot! 👍 👍 👍
Let me merge this after build with Gradle 9.0, which isn't release yet, to make sure if we don't miss anything.

@sh-cho
Create project directories for :bom, :version-catalog
f7d6290 
Motivation:

Starting with Gradle 9.0, Gradle will not run builds if a project
directory is missing or read-only.
(ref:
https://docs.gradle.org/8.7/userguide/upgrading_version_8.html#deprecated_missing_project_directory)

Modification:

- Create project directories
  - `:bom`, `:version-catalog`
  - `:dependencyManagement` will be handled via
    line/gradle-scripts#177

Result:

- Deprecation for Gradle 9.0 is prepared for this behavior
@sh-cho
Set project directory under build/virtual-projects
9dc8494 
This will creates project directory under there, not under Root.
@sh-cho sh-cho force-pushed the gradle-missing-project-dir branch from 12edadf to 9dc8494 Compare May 20, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikhoon ikhoon ikhoon left review comments

@minwoox minwoox minwoox approved these changes

@trustin trustin Awaiting requested review from trustin trustin is a code owner

@jrhee17 jrhee17 Awaiting requested review from jrhee17 jrhee17 is a code owner

Assignees
No one assigned
Labels
dependencies
Projects
None yet
Milestone
1.29.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@sh-cho @trustin @ikhoon @jrhee17 @minwoox