-
Notifications
You must be signed in to change notification settings - Fork 20
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
don't fail if fuzzOut isn't specified #413
Conversation
@@ -13,5 +13,7 @@ dependencies { | |||
|
|||
tasks.register<Copy>("copyToFuzzOut") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The task should be Sync
rather than Copy
to remove stale files if some of the classes are removed from build to build
fuzzing/build.gradle.kts
Outdated
into(project.property("fuzzOut")).from(sourceSets.main.get().runtimeClasspath) | ||
val defaultDestination = project.layout.buildDirectory.dir("fuzzOut") | ||
val dest = if (project.hasProperty("fuzzOut")) project.property("fuzzOut") ?: defaultDestination else defaultDestination | ||
into(dest).from(sourceSets.main.get().runtimeClasspath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
into(dest).from(sourceSets.main.get().runtimeClasspath) | |
into(dest) | |
from(sourceSets.main.get().runtimeClasspath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this more idiomatic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so
They describe two different things:
- where to store the data
- which data to pick
For instance, if you would ever add the second from
, then into(...).from(..).from(..)
would look too long, while the following is fine:
into(...)
from(...)
from(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider this:
into(dest)
into("abcd") {
from(sourceSets.main.get().runtimeClasspath)
}
=>
% tree build/fuzzOut
build/fuzzOut
└── abcd
├── animal-sniffer-annotations-1.21.jar
├── annotations-4.1.1.4.jar
├── bcpkix-jdk18on-1.72.jar
├── bcprov-jdk18on-1.72.jar
4b5a721
to
8b9c054
Compare
hrmm.. I messed something up |
8b9c054
to
a23b2e3
Compare
ah we can't use Sync because the fuzzer builder is putting other things in the directory before gradle executes |
Copying is inherently unsafe (sometime later one might end up with multiple bc-java jars having slightly different versions), so I would prefer: or b) document the reasons for having |
Signed-off-by: Appu Goundan <appu@google.com>
a23b2e3
to
4e2dee7
Compare
I mean (a) seems a bit much, we know it's a clean build, so the concerns don't exactly apply. Yes technically that could happen, but it wont. Added in some comments. |
fixes #412