The generated artifacts include source and java-docs. I am wondering if the rule is required for that purpose. We can test removing it and build artifacts and make sure the java-doc artifacts is ok to keep things simple.
If it is not required by artifacts, I feel we can consider removing it to keep things simple. On Sat, Jun 13, 2020 at 7:17 PM Josh Fischer <j...@joshfischer.io> wrote: > Hi All, > > Sending this again: > > * TLDR; > There is nothing wrong with the IDE setup script. We have a custom > java_doc rule in the code base that is broken since the java 11 upgrade. > We have several bazel targets in the code base that reference this java_doc > rule implemented in the heron code base which causes the setup IDE script > to fail. The build runs successfully, but the rule outputs get put in the > wrong package. My question is: Do we use these java_doc rules for > anything? The website build script generates java docs in a different > way. Can we remove this rule and corresponding targets that reference said > rule or do we try and fix the rule? What value do we get from fixing the > rule? What do we lose if we take the rule out of the code base? > > Basically we are requiring Bazel to generate java docs just to open Heron > in an IDE. I do not feel this right. > > > ** The longer explanation/rambling..... > > I did some looking into the setup script issue. Basically the java_doc > rule is referring to the "java" folder as one of the packages and causing > the IDE setup script to fail. So instead of seeing the package > "org.apache.heron" it sees "java.org.apache.heron" and etc. This > misalignment of packages causes the IDE setup script to run unsuccessfully. > > If you look below I'll copy and paste a few snippets of the build output > that show that it cannot find the correct class in the jar. All classes > not found are the ones in referenced in the pull request above. > > the command I executed was > > $ ./scripts/setup_intellij.sh > > Some of the failing targets were below. The setup script is not failing or > incorrect. It's the jar that is created from the javadocs that cause the > issue. The javadoc jar classes have "java" prepended in their package > names which is what I'm assuming is throwing off the reverse look up for > targets from outputs. The classes in the actual storm compatibility jar > have the correct packages. > > Couldn't get generating target for > > :/storm-compatibility/src/java/heron-storm-javadoc.zip.source/java/storm/trident/spout/ISpoutPartition.java > > scripts/get_all_heron_paths.sh: line 21: ./output/bazel: No such file or > directory > > scripts/get_all_heron_paths.sh: line 21: ./output/bazel: No such file or > directory > > Couldn't get generating target for > > :/storm-compatibility/src/java/heron-storm-javadoc.zip.source/java/storm/trident/state/Serializer.java > > sample classes from the storm compatibility jar is below: > > jar -tf libstorm-compatibility-java.jar > > META-INF/ > > META-INF/MANIFEST.MF > > backtype/ > > backtype/storm/ > > backtype/storm/Config.class > > backtype/storm/Constants.class > > backtype/storm/ILocalCluster.class > > backtype/storm/LocalCluster.class > > backtype/storm/StormSubmitter.classic.class > > backtype/storm/utils/ConfigUtils.class > > backtype/storm/utils/DefaultMaxSpoutPendingTuner.class > > backtype/storm/utils/ListDelegate.class > > backtype/storm/utils/Utils.class > > clojure/ > > clojure/lang/ > > clojure/lang/Atom.class > > org/ > > org/apache/ > > org/apache/storm/ > > org/apache/storm/Config.class > > org/apache/storm/Constants.class > > org/apache/storm/ILocalCluster.class > > org/apache/storm/LocalCluster.class > > org/apache/storm/StormSubmitter.class > > org/apache/storm/topology/ > > org/apache/storm/topology/BasicBoltExecutor.class > > org/apache/storm/topology/BasicOutputCollector.class > > org/apache/storm/topology/BoltDeclarer.class > > org/apache/storm/topology/BoltDeclarerImpl.class > > org/apache/storm/trident/spout/ > > org/apache/storm/trident/spout/ISpoutPart > > > > > output from the java_doc rule zip is below. If you look below the packages > "backtype", "storm", and "clojure" are under the "java" folder which is > incorrect. They should be structured like the files under the "org" > folder. > > heron-storm-javadoc.zip.dir joshfischer$ tree > > . > > ├── allclasses-index.html > > ├── allclasses.html > > ├── allpackages-index.html > > ├── constant-values.html > > ├── deprecated-list.html > > ├── element-list > > ├── help-doc.html > > ├── index-all.html > > ├── index.html > > ├── java > > │ ├── backtype > > │ │ └── storm > > │ │ ├── generated > > │ │ │ ├── package-summary.html > > │ │ │ └── package-tree.html > > │ │ ├── grouping > > │ │ │ ├── package-summary.html > > │ │ │ └── package-tree.html > > │ ├── clojure > > │ │ └── lang > > │ │ ├── package-summary.html > > │ │ └── package-tree.html > > │ └── storm > > │ └── trident > > │ ├── spout > > │ │ ├── package-summary.html > > │ │ └── package-tree.html > > │ └── state > > │ ├── package-summary.html > > │ └── package-tree.html > > > ├── org > > │ └── apache > > │ └── storm > > │ ├── Config.html > > │ ├── Constants.html > > │ ├── ILocalCluster.html > > │ ├── LocalCluster.html > > │ ├── StormSubmitter.html > > │ ├── clojure > > │ │ └── lang > > │ │ ├── Atom.html > > │ │ ├── package-summary.html > > │ │ └── package-tree.html > > On Thu, Jun 11, 2020 at 7:15 PM Josh Fischer <j...@joshfischer.io> wrote: > > > Hi All, > > > > * TLDR; > > There is nothing wrong with the IDE setup script. We have a custom > > java_doc rule in the code base that is broken since the java 11 upgrade. > > We have several bazel targets in the code base that reference this > java_doc > > rule implemented in the heron code base which causes the setup IDE script > > to fail. The build runs successfully, but classes get put in the wrong > > package. My question is: Do we use these java_doc rules for anything? > The > > website build script generates java docs in a different way. Can we > remove > > this rule and corresponding targets that reference said rule or do we try > > and fix the rule? What value do we get from fixing the rule? What do we > > lose if we take it out of the code base? Basically we are requiring > Bazel > > to generate java docs just to open Heron. I do not feel this right. > > > > > > ** The longer explanation/rambling..... > > > > I did some looking into the setup script issue. Basically the java_doc > > rule is referring to the "java" folder as one of the packages and causing > > the build to break. So instead of seeing the package "org.apache.heron" > it > > sees "java.org.apache.heron" and etc. This misalignment of packages > causes > > the IDE setup script to run unsuccessfully. > > > > If you look below I'll copy and paste a few snippets of the build output > > that show that it cannot find the correct class in the jar. All classes > > not found are the ones in referenced in the pull request above. > > > > the command I executed was > > > > $ ./scripts/setup_intellij.sh > > > > Some of the failing targets were below. The setup script is not failing > > or incorrect. It's the jar that is created from the javadocs that cause > > the issue. The javadoc jar classes have "java" prepended in their > package > > names which is what I'm assuming is throwing off the reverse look up for > > targets from outputs. The classes in the actual storm compatibility jar > > have the correct packages. > > > > Couldn't get generating target for > > > :/storm-compatibility/src/java/heron-storm-javadoc.zip.source/java/storm/trident/spout/ISpoutPartition.java > > > > scripts/get_all_heron_paths.sh: line 21: ./output/bazel: No such file or > > directory > > > > scripts/get_all_heron_paths.sh: line 21: ./output/bazel: No such file or > > directory > > > > Couldn't get generating target for > > > :/storm-compatibility/src/java/heron-storm-javadoc.zip.source/java/storm/trident/state/Serializer.java > > > > sample classes from the storm compatibility jar is below: > > > > jar -tf libstorm-compatibility-java.jar > > > > META-INF/ > > > > META-INF/MANIFEST.MF > > > > backtype/ > > > > backtype/storm/ > > > > backtype/storm/Config.class > > > > backtype/storm/Constants.class > > > > backtype/storm/ILocalCluster.class > > > > backtype/storm/LocalCluster.class > > > > backtype/storm/StormSubmitter.classic.class > > > > backtype/storm/utils/ConfigUtils.class > > > > backtype/storm/utils/DefaultMaxSpoutPendingTuner.class > > > > backtype/storm/utils/ListDelegate.class > > > > backtype/storm/utils/Utils.class > > > > clojure/ > > > > clojure/lang/ > > > > clojure/lang/Atom.class > > > > org/ > > > > org/apache/ > > > > org/apache/storm/ > > > > org/apache/storm/Config.class > > > > org/apache/storm/Constants.class > > > > org/apache/storm/ILocalCluster.class > > > > org/apache/storm/LocalCluster.class > > > > org/apache/storm/StormSubmitter.class > > > > org/apache/storm/topology/ > > > > org/apache/storm/topology/BasicBoltExecutor.class > > > > org/apache/storm/topology/BasicOutputCollector.class > > > > org/apache/storm/topology/BoltDeclarer.class > > > > org/apache/storm/topology/BoltDeclarerImpl.class > > > > org/apache/storm/trident/spout/ > > > > org/apache/storm/trident/spout/ISpoutPart > > > > > > > > > > output from the java_doc rule zip is below. If you look below the > > packages "backtype", "storm", and "clojure" are under the "java" folder > > which is incorrect. They should be structured like the files under the > > "org" folder. > > > > heron-storm-javadoc.zip.dir joshfischer$ tree > > > > . > > > > ├── allclasses-index.html > > > > ├── allclasses.html > > > > ├── allpackages-index.html > > > > ├── constant-values.html > > > > ├── deprecated-list.html > > > > ├── element-list > > > > ├── help-doc.html > > > > ├── index-all.html > > > > ├── index.html > > > > ├── java > > > > │ ├── backtype > > > > │ │ └── storm > > > > │ │ ├── generated > > > > │ │ │ ├── package-summary.html > > > > │ │ │ └── package-tree.html > > > > │ │ ├── grouping > > > > │ │ │ ├── package-summary.html > > > > │ │ │ └── package-tree.html > > > > │ ├── clojure > > > > │ │ └── lang > > > > │ │ ├── package-summary.html > > > > │ │ └── package-tree.html > > > > │ └── storm > > > > │ └── trident > > > > │ ├── spout > > > > │ │ ├── package-summary.html > > > > │ │ └── package-tree.html > > > > │ └── state > > > > │ ├── package-summary.html > > > > │ └── package-tree.html > > > > > > ├── org > > > > │ └── apache > > > > │ └── storm > > > > │ ├── Config.html > > > > │ ├── Constants.html > > > > │ ├── ILocalCluster.html > > > > │ ├── LocalCluster.html > > > > │ ├── StormSubmitter.html > > > > │ ├── clojure > > > > │ │ └── lang > > > > │ │ ├── Atom.html > > > > │ │ ├── package-summary.html > > > > │ │ └── package-tree.html > > > > On Wed, Jun 10, 2020 at 11:49 AM Ning Wang <wangnin...@gmail.com> wrote: > > > >> Thanks! > >> > >> On Wed, Jun 10, 2020 at 5:49 AM Josh Fischer <j...@joshfischer.io> > wrote: > >> > >> > Hi All, > >> > > >> > The javadoc issue should be fixed on both the website build and in the > >> code > >> > base build. I believe we have one more issue to fix before we start > the > >> > next release candidate vote. The last issue I can think of is fixing > >> the > >> > IDE setup scripts. I have an idea of how to fix them. It seems that > >> Bazel's > >> > behavior has changed over time resulting in the setup scripts to fail > >> while > >> > doing a reverse lookup on targets when looking at a source file. Once > >> we > >> > have this fixed I think we are good to go. > >> > > >> > Just a note, it might be helpful to note in the next vote to note in > the > >> > email that it takes anywhere from 30 - 45 minutes on a fresh build to > >> > setup eclipse or intellij with Heron. It might save some people some > >> time > >> > during the vote if they do not wish to wait that long to view the > >> codebase > >> > in an IDE. > >> > > >> > - Josh > >> > > >> > > >
