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 <kl...@apache.org> 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 <gosulli...@pivotal.io>
> 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 <kl...@apache.org> 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 <jbarr...@pivotal.io>
> > >
> > > 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 <fsoutherl...@pivotal.io>
> > >
> > > Thanks,
> > > Kirk
> > >
> >
>
