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

Append feature for p2AsMaven #44

Merged
merged 4 commits into from
Dec 19, 2017
Merged

Append feature for p2AsMaven #44

merged 4 commits into from
Dec 19, 2017

Conversation

hacki11
Copy link
Contributor

@hacki11 hacki11 commented Dec 14, 2017

this is the pr for #43

@@ -68,7 +68,6 @@ public void run() throws Exception {
project.getLogger().lifecycle("p2AsMaven " + def.group + " is dirty.");
}
// else, we'll need to run our own little thing
FileMisc.cleanDir(dirP2());
FileMisc.cleanDir(dirP2Runnable());
Copy link
Member

Choose a reason for hiding this comment

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

Should this be guarded on append? Otherwise this looks great and ready to merge to me!

@hacki11
Copy link
Contributor Author

hacki11 commented Dec 14, 2017

I am only aware of the native p2.mirror append feature. Do you know wether Repo2Runnable is appendable? It could end in a corrupt repo db but i did not verify this. The maven install perhaps can handle it.

With just keeping the original p2 dir we can guarantee a consistent state in all three directories.

@hacki11
Copy link
Contributor Author

hacki11 commented Dec 14, 2017

The guard around clean of p2dir is not needed. p2.mirror clears it automatically if append = false

@nedtwigg
Copy link
Member

There's nothing in the docs for repo2runnable that mentions appendable. MavenInstall could be made to handle appendable without much trouble.

My experience is that the vast majority of the time is spent in the p2 mirroring, so I don't think it's worth spending much effort on the much faster repo2runnable and mavenInstall phases. And it can always be done later :)

If it looks good to you, I'll merge this now. Oxygen.2 releases tomorrow, so I'll add support for that then cut a new release of Goomph which includes your improvement here and Oxygen.2.

@hacki11
Copy link
Contributor Author

hacki11 commented Dec 19, 2017

the only thing is every clean existing p2AsMaven mirror will be "dirty" the first time one gets the new version because of the implemented token mechanism. this was needed to keep the default behavior append=false.

@nedtwigg
Copy link
Member

Gotcha. Gradle forces everything to run again when the plugin classpath changes anyway, so I think that's alright.

@nedtwigg nedtwigg merged commit 5a9e251 into diffplug:master Dec 19, 2017
@nedtwigg
Copy link
Member

Released in 3.9.0.

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.

2 participants