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

Restore compatibility with Gradle's configuration cache #96

Merged
merged 2 commits into from Aug 31, 2022
Merged

Restore compatibility with Gradle's configuration cache #96

merged 2 commits into from Aug 31, 2022

Conversation

3flex
Copy link
Contributor

@3flex 3flex commented Jul 25, 2022

Fixes #95

@qwwdfsad qwwdfsad self-requested a review August 15, 2022 16:35
Copy link
Member

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of it!

I have a question about mkdirs and this PR is good to go

group = "other"
description = "Syncs API from build dir to ${targetConfig.apiDir} dir for $projectName"
from(apiBuildDir)
into(apiCheckDir)
dependsOn(apiBuild)
doFirst {
Copy link
Member

Choose a reason for hiding this comment

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

Could you please elaborate on why you removed this mkdirs call?

Does into documentation guarantees mkdir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's odd, as this doesn't seem to violate the configuration cache contract, but it wouldn't work unless I removed it.

In any case, yes, into will create the required directories. More here: https://docs.gradle.org/current/userguide/working_with_files.html#sec:creating_directories_example

All core Gradle tasks ensure that any output directories they need are created if necessary

Copy link
Member

Choose a reason for hiding this comment

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

Works for me, thanks!
At least now we have a proper test coverage for that

@qwwdfsad qwwdfsad self-requested a review August 31, 2022 09:16
Copy link
Member

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

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

Once again, thanks for taking care of it!

@qwwdfsad qwwdfsad merged commit b569f24 into Kotlin:master Aug 31, 2022
@3flex 3flex deleted the restore-cc branch August 31, 2022 11:45
@3flex
Copy link
Contributor Author

3flex commented Sep 2, 2022

Any chance we could get a new release cut? Not sure if there's anything else planned for the next version, but I'd really like to get rid of the notCompatibleWithConfigurationCache call for these tasks in the build scripts!

@qwwdfsad
Copy link
Member

qwwdfsad commented Sep 5, 2022

0.11.1 with the fix is published to maven central. Should be available for regular dependencies within an hour

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.

apiDump task incompatible with configuration cache
2 participants