search

wooga / atlas-github

Gradle plugin to publish artifacts to github
https://wooga.github.io/atlas-github/
Apache License 2.0
1 stars 3 forks source link

Better temp-file handling #12

Closed Vampire closed 6 years ago

Vampire commented 6 years ago

https://github.com/wooga/atlas-github/blob/72012b690cda7cb359f37969053458c8d9947b26/src/main/groovy/wooga/gradle/github/publish/tasks/GithubPublish.groovy#L80-L83

is maybe not the best idea. Depending on deleteOnExit can easily lead to stale files if the daemon VM crashes or is killed and each task has a designated temporary directory. So maybe it would be better to use

assetCollectDirectory = project.file("$temporaryDir/collect")
assetUploadDirectory = project.file("$temporaryDir/prepare")

And then project.sync instead of project.copy for filling the assetCollectDirectory and deleting the assetUploadDirectory before copying over the files thereto, letting the clean task delete the directory as usual. This also allows to better look for problems, if the files are not deleted automatically after the VM shuts down.

Larusso commented 6 years ago

So to understand correctly. Each task should create one temp directory called temporaryDir? And tanks again. It was not my intention to rely only on deleteOnExit. I thought I added a second delete logic ... Your proposal makes perfect sense.

Vampire commented 6 years ago

Oh, maybe i missed the second delete logic, I just skimmed over your code and the others that are similar to quickly evaluate which I try first. :-)

Each task automatically has a temporary directory assigned by Gradle. They are in $buildDir/tmp/${task.name}. They are not created if you don't use them. But you can easily use them by using the getTemporaryDir() method and thus the temporaryDir property of the DefaultTask.

Larusso commented 6 years ago

Ah cool thanks. Didn't know that. Then it makes total sense to use this instead because clean will cover it without further configuration.

Vampire commented 6 years ago

Exactly my point. :-)