Re: Two copies of ExtendsFunctionAdapter.java
I agree that the Gradle is pathological in a lot of places. I've been
working on correcting that for the last several months. It's slow going
when you're restricted from breaking everything for a while. But it is
getting better, for what it's worth.
I can't tell from that stack-blob where you hit an error -- in a test or in
a build -- but I agree that resources shouldn't be getting compiled.
That's a separate bit of strange. Maybe new IntelliJ is doing extra work
to make sure there are no filename collisions? But I agree it's strange to
have it duplicated, in any event.
I don't know if we need two copies of the file. I would assume that we
have one in geode-core:integrationTest:resources and one in
geode-core:distributedTest:resources because the file is needed in at least
one integration test and at least one distributed test. If it's there as a
resource and not in the source, I would guess that it's for deploy command
testing, in which we want to make sure the servers don't already have at
launch the class that is being deployed.
The geode-dunit module exists for those resources that belong to that
intersection of integration and distributed tests. Perhaps everything
would be fine if a single copy lived in geode-dunit:main:resources. If you
wanted to look into it, I'd suggest starting there.
Imagination is Change.
~Patrick
On Mon, Dec 17, 2018 at 12:26 PM Kirk Lund wrote:
> You're right. IntelliJ shouldn't be compiling these classes. However, the
> real issue here is that our use of Gradle is so horribly unnatural that it
> ends up confusing IntelliJ (and other tools). We should strive to NOT
> confuse other tools by doing stupid stuff in Gradle.
>
> You've seen my instructions for configuring IJ and setting up a project (or
> I assume you have). I'm still following that for every Geode project I
> setup. Sometimes, one of these projects "misbehaves" and gets so confused
> by Geode's crazy Gradle build that it goes belly-up. The result is we are
> wasting the time of lots of developers because of this kind of non-sense.
>
> So, I ask again: do we really need two copies of the same file?
>
> On Mon, Dec 17, 2018 at 11:49 AM Galen O'Sullivan
> wrote:
>
> > I don't understand our build or IntelliJ that well, but it seems weird to
> > me that these classes would be getting built at all since they're in the
> > resources section rather than java. I don't see compiled versions of
> these
> > classes in my geode directory. Perhaps it's an IntelliJ configuration
> > issue?
> >
> > Galen
> >
> >
> > On Mon, Dec 17, 2018 at 11:23 AM Kirk Lund wrote:
> >
> > > IntelliJ just started failing to compile because we have two copies of
> > > ExtendsFunctionAdapter.java. Apparently, IJ was happy enough to ignore
> > > these duplicates for a month or so, but it's now fed up and will no
> > longer
> > > tolerate the duplication so it's failing with:
> > >
> > > Error:(21, 8) java: duplicate class:
> > > org.apache.geode.management.internal.deployment.ExtendsFunctionAdapter
> > >
> > > This file is in geode-core/src/distributedTest/resources and
> > > geode-core/src/integrationTest/resources:
> > >
> > > 1)
> > >
> > >
> >
> ./geode-core/src/distributedTest/resources/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java
> > > 2)
> > >
> > >
> >
> ./geode-core/src/integrationTest/resources/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java
> > >
> > > Apparently we have two tests that load these java files as resources:
> > >
> > > 1)
> > >
> > >
> >
> geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DeployCommandFunctionRegistrationDUnitTest.java:83:
> > >
> > >
> > >
> >
> "/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java");
> > > 2.a)
> > >
> > >
> >
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:62:
> > > File sourceFileOne = loadTestResource("ExtendsFunctionAdapter.java");
> > > 2.b)
> > >
> > >
> >
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:73:
> > > File sourceFileOne =
> > > loadTestResource("AbstractExtendsFunctionAdapter.java");
> > > 2.c)
> > >
> > >
> >
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:74:
> >
Re: Two copies of ExtendsFunctionAdapter.java
You're right. IntelliJ shouldn't be compiling these classes. However, the
real issue here is that our use of Gradle is so horribly unnatural that it
ends up confusing IntelliJ (and other tools). We should strive to NOT
confuse other tools by doing stupid stuff in Gradle.
You've seen my instructions for configuring IJ and setting up a project (or
I assume you have). I'm still following that for every Geode project I
setup. Sometimes, one of these projects "misbehaves" and gets so confused
by Geode's crazy Gradle build that it goes belly-up. The result is we are
wasting the time of lots of developers because of this kind of non-sense.
So, I ask again: do we really need two copies of the same file?
On Mon, Dec 17, 2018 at 11:49 AM Galen O'Sullivan
wrote:
> I don't understand our build or IntelliJ that well, but it seems weird to
> me that these classes would be getting built at all since they're in the
> resources section rather than java. I don't see compiled versions of these
> classes in my geode directory. Perhaps it's an IntelliJ configuration
> issue?
>
> Galen
>
>
> On Mon, Dec 17, 2018 at 11:23 AM Kirk Lund wrote:
>
> > IntelliJ just started failing to compile because we have two copies of
> > ExtendsFunctionAdapter.java. Apparently, IJ was happy enough to ignore
> > these duplicates for a month or so, but it's now fed up and will no
> longer
> > tolerate the duplication so it's failing with:
> >
> > Error:(21, 8) java: duplicate class:
> > org.apache.geode.management.internal.deployment.ExtendsFunctionAdapter
> >
> > This file is in geode-core/src/distributedTest/resources and
> > geode-core/src/integrationTest/resources:
> >
> > 1)
> >
> >
> ./geode-core/src/distributedTest/resources/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java
> > 2)
> >
> >
> ./geode-core/src/integrationTest/resources/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java
> >
> > Apparently we have two tests that load these java files as resources:
> >
> > 1)
> >
> >
> geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DeployCommandFunctionRegistrationDUnitTest.java:83:
> >
> >
> >
> "/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java");
> > 2.a)
> >
> >
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:62:
> > File sourceFileOne = loadTestResource("ExtendsFunctionAdapter.java");
> > 2.b)
> >
> >
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:73:
> > File sourceFileOne =
> > loadTestResource("AbstractExtendsFunctionAdapter.java");
> > 2.c)
> >
> >
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:74:
> > File sourceFileTwo =
> > loadTestResource("ConcreteExtendsAbstractExtendsFunctionAdapter.java");
> >
> > Do we really need to have two copies of this file in our codebase?
> >
> > PS, here's the last commit to touch these two files:
> >
> > commit 65c79841b65d7bd9ffa3c50fa73d4d3857dced58
> >
> > Author: Jacob Barrett
> >
> > Date: Fri Aug 10 15:49:22 2018 -0700
> >
> >
> > GEODE-5530: Removes test dependency from other test source sets
> > (#2294)
> >
> >
> >
> > Moves common sources to geode-dunit or geode-junit.
> >
> >
> >
> > Co-authored-by: Finn Sutherland
> >
> > Thanks,
> > Kirk
> >
>
Re: Two copies of ExtendsFunctionAdapter.java
I don't understand our build or IntelliJ that well, but it seems weird to
me that these classes would be getting built at all since they're in the
resources section rather than java. I don't see compiled versions of these
classes in my geode directory. Perhaps it's an IntelliJ configuration issue?
Galen
On Mon, Dec 17, 2018 at 11:23 AM Kirk Lund wrote:
> IntelliJ just started failing to compile because we have two copies of
> ExtendsFunctionAdapter.java. Apparently, IJ was happy enough to ignore
> these duplicates for a month or so, but it's now fed up and will no longer
> tolerate the duplication so it's failing with:
>
> Error:(21, 8) java: duplicate class:
> org.apache.geode.management.internal.deployment.ExtendsFunctionAdapter
>
> This file is in geode-core/src/distributedTest/resources and
> geode-core/src/integrationTest/resources:
>
> 1)
>
> ./geode-core/src/distributedTest/resources/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java
> 2)
>
> ./geode-core/src/integrationTest/resources/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java
>
> Apparently we have two tests that load these java files as resources:
>
> 1)
>
> geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DeployCommandFunctionRegistrationDUnitTest.java:83:
>
>
> "/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java");
> 2.a)
>
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:62:
> File sourceFileOne = loadTestResource("ExtendsFunctionAdapter.java");
> 2.b)
>
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:73:
> File sourceFileOne =
> loadTestResource("AbstractExtendsFunctionAdapter.java");
> 2.c)
>
> geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:74:
> File sourceFileTwo =
> loadTestResource("ConcreteExtendsAbstractExtendsFunctionAdapter.java");
>
> Do we really need to have two copies of this file in our codebase?
>
> PS, here's the last commit to touch these two files:
>
> commit 65c79841b65d7bd9ffa3c50fa73d4d3857dced58
>
> Author: Jacob Barrett
>
> Date: Fri Aug 10 15:49:22 2018 -0700
>
>
> GEODE-5530: Removes test dependency from other test source sets
> (#2294)
>
>
>
> Moves common sources to geode-dunit or geode-junit.
>
>
>
> Co-authored-by: Finn Sutherland
>
> Thanks,
> Kirk
>
Two copies of ExtendsFunctionAdapter.java
IntelliJ just started failing to compile because we have two copies of
ExtendsFunctionAdapter.java. Apparently, IJ was happy enough to ignore
these duplicates for a month or so, but it's now fed up and will no longer
tolerate the duplication so it's failing with:
Error:(21, 8) java: duplicate class:
org.apache.geode.management.internal.deployment.ExtendsFunctionAdapter
This file is in geode-core/src/distributedTest/resources and
geode-core/src/integrationTest/resources:
1)
./geode-core/src/distributedTest/resources/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java
2)
./geode-core/src/integrationTest/resources/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java
Apparently we have two tests that load these java files as resources:
1)
geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DeployCommandFunctionRegistrationDUnitTest.java:83:
"/org/apache/geode/management/internal/deployment/ExtendsFunctionAdapter.java");
2.a)
geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:62:
File sourceFileOne = loadTestResource("ExtendsFunctionAdapter.java");
2.b)
geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:73:
File sourceFileOne =
loadTestResource("AbstractExtendsFunctionAdapter.java");
2.c)
geode-core/src/integrationTest/java/org/apache/geode/management/internal/deployment/FunctionScannerTest.java:74:
File sourceFileTwo =
loadTestResource("ConcreteExtendsAbstractExtendsFunctionAdapter.java");
Do we really need to have two copies of this file in our codebase?
PS, here's the last commit to touch these two files:
commit 65c79841b65d7bd9ffa3c50fa73d4d3857dced58
Author: Jacob Barrett
Date: Fri Aug 10 15:49:22 2018 -0700
GEODE-5530: Removes test dependency from other test source sets (#2294)
Moves common sources to geode-dunit or geode-junit.
Co-authored-by: Finn Sutherland
Thanks,
Kirk
