You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@heron.apache.org by GitBox <gi...@apache.org> on 2020/02/03 22:12:37 UTC
[GitHub] [incubator-heron] joshfischer1108 opened a new pull request #3444:
Fixing setup script: Do Not Merge
joshfischer1108 opened a new pull request #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444
This is my attempt to fix the IDE setup script. I commented out some code that was breaking the initialization script when starting both intellij and eclipse. I can understand what the code is doing, but I'm not sure why it is needed for loading Heron into an IDE. I'll as of now I can run the script, it compiles Heron and opens up Intellij for me without errors. I'm double checking and asking for feedback to make sure I didn't comment out any needed code. Please review.
Below is my output on a mac.
I check that the iml file exists, delete the `.iml` file from the folder, run the script and check that the `.iml` file has been created again. The IDE also opens up for me and seems to be working as expected.
```
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ ls | grep heron.iml
heron.iml
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ rm heron.iml
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ ./scripts/setup-intellij.sh
changing to /Users/joshfischer/Source/apache/incubator-heron/scripts/..
WARNING: /Users/joshfischer/Source/apache/incubator-heron/heron/healthmgr/tests/java/BUILD:52:12: in srcs attribute of java_library rule //heron/healthmgr/tests/java:healthmgr-tests: please do not import '//heron/healthmgr/src/java:org/apache/heron/healthmgr/HealthManager.java' directly. You should either move the file to this package or depend on an appropriate rule there
INFO: Analyzed 759 targets (0 packages loaded, 0 targets configured).
INFO: Found 759 targets...
INFO: Elapsed time: 0.465s, Critical Path: 0.06s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
Bazel build successful!!
Path is /Users/joshfischer/Source/apache/incubator-heron
Generating IDEA project...
Loading: 0 packages loaded
Done. IDEA module file: heron.iml
Opening Heron project in IDEA...
Done.
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ ls | grep heron.iml
heron.iml
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 edited a comment on issue #3444:
Fixing IDE setup script with Bazel: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 edited a comment on issue #3444: Fixing IDE setup script with Bazel: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#issuecomment-582712525
@nwangtw `//heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java` points to a file in one of my local Bazel build output directories. I think I'm going to hold off on this until we get Bazel upgraded to 2.0.0 to complete.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 commented on issue #3444: Fixing
setup script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on issue #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#issuecomment-582166917
> Without knowing much about this script, my guess is that it generates libraries first and then searches for targets depending on the libraries and then creates IDE targets accordingly. So removing the lines could work but some targets will be missing in the IDE.
I'm sure you are probably correct. I'm guessing 0.26.0 broke something with the query syntax in previous versions. Back to the drawing board!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 commented on issue #3444: Fixing
setup script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on issue #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#issuecomment-581926065
@nicknezis @simingweng It might be good if you looked at this as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 edited a comment on issue #3444:
Fixing setup script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 edited a comment on issue #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#issuecomment-582192974
@nwangtw It's interesting.. See the command that fails during the setup intellij script is when the script executes `bazel query "something"`. In this particular case I can open up and see the file it's complaining about in the IDE. Bazel is searching for Bazel targets containing a file, but I'm not sure how it translates to an IDE yet. Thoughts?
```
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ bazel query 'kind(rule, deps(//heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java, 1)) - //heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java'
ERROR: no such target '//heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java': target 'heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java' not declared in package 'heron/spi/src/java' defined by /Users/joshfischer/Source/apache/incubator-heron/heron/spi/src/java/BUILD
Loading: 1 packages loaded
```
If you look at the output above the command claims `Error: no such target`, but then at the bottom of the output you can see it loaded 1 package.
```
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ bazel query 'kind(rule, deps(//heron/spi/src/java/org/apache/heron/spi/statemgr/IStateManager.java, 1)) - //heron/spi/src/java/org/apache/heron/spi/statemgr/IStateManager.java'
ERROR: no such package 'heron/spi/src/java/org/apache/heron/spi/statemgr/IStateManager.java': BUILD file not found on package path
Loading: 0 packages loaded
```
Here I intentionally changed the directory path to make it break. It also claims `Error: no such target`. We can also see at the bottom of this script 0 packages loaded.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 closed pull request #3444: Fixing
IDE setup script with Bazel: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 closed pull request #3444: Fixing IDE setup script with Bazel: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 commented on issue #3444: Fixing
setup script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on issue #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#issuecomment-582192974
@nwangtw It's interesting.. See the command that fails during the setup intellij script is when the script executes `bazel query "something"`. In this particular case I can open up and see the file it's complaining about in the IDE. Thoughts?
```
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ bazel query 'kind(rule, deps(//heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java, 1)) - //heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java'
ERROR: no such target '//heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java': target 'heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java' not declared in package 'heron/spi/src/java' defined by /Users/joshfischer/Source/apache/incubator-heron/heron/spi/src/java/BUILD
Loading: 1 packages loaded
```
If you look at the output above the command claims `Error: no such target`, but then at the bottom of the output you can see it loaded 1 package.
```
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ bazel query 'kind(rule, deps(//heron/spi/src/java/org/apache/heron/spi/statemgr/IStateManager.java, 1)) - //heron/spi/src/java/org/apache/heron/spi/statemgr/IStateManager.java'
ERROR: no such package 'heron/spi/src/java/org/apache/heron/spi/statemgr/IStateManager.java': BUILD file not found on package path
Loading: 0 packages loaded
```
Here I intentionally changed the directory path to make it break. It also claims `Error: no such target`. We can also see at the bottom of this script 0 packages loaded.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 commented on issue #3444: Fixing
IDE setup script with Bazel: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 commented on issue #3444: Fixing IDE setup script with Bazel: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#issuecomment-582712525
@nwangtw `//heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java` points to a file in one of my local Bazel build output directories. I think I'm going to hold off on this until we get Bazel upgraded to 2.0.0 before attempting to fix this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 opened a new pull request #3444:
Fixing setup script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 opened a new pull request #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444
This is my attempt to fix the IDE setup script. I commented out some code that was breaking the initialization script when starting both intellij and eclipse. I can understand what the code is doing, but I'm not sure why it is needed for loading Heron into an IDE. I'll as of now I can run the script, it compiles Heron and opens up Intellij for me without errors. I'm double checking and asking for feedback to make sure I didn't comment out any needed code. Please review.
Below is my output on a mac.
I check that the iml file exists, delete the `.iml` file from the folder, run the script and check that the `.iml` file has been created again. The IDE also opens up for me and seems to be working as expected.
```
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ ls | grep heron.iml
heron.iml
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ rm heron.iml
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ ./scripts/setup-intellij.sh
changing to /Users/joshfischer/Source/apache/incubator-heron/scripts/..
WARNING: /Users/joshfischer/Source/apache/incubator-heron/heron/healthmgr/tests/java/BUILD:52:12: in srcs attribute of java_library rule //heron/healthmgr/tests/java:healthmgr-tests: please do not import '//heron/healthmgr/src/java:org/apache/heron/healthmgr/HealthManager.java' directly. You should either move the file to this package or depend on an appropriate rule there
INFO: Analyzed 759 targets (0 packages loaded, 0 targets configured).
INFO: Found 759 targets...
INFO: Elapsed time: 0.465s, Critical Path: 0.06s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
Bazel build successful!!
Path is /Users/joshfischer/Source/apache/incubator-heron
Generating IDEA project...
Loading: 0 packages loaded
Done. IDEA module file: heron.iml
Opening Heron project in IDEA...
Done.
Joshs-MacBook-Pro-2:incubator-heron joshfischer$ ls | grep heron.iml
heron.iml
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] nwangtw commented on issue #3444: Fixing setup
script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
nwangtw commented on issue #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#issuecomment-582006202
Without knowing much about this script, my guess is that it generates libraries first and then searches for targets depending on the libraries and then creates IDE targets accordingly. So removing the lines could work but some targets will be missing in the IDE.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] nwangtw commented on a change in pull request
#3444: Fixing setup script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
nwangtw commented on a change in pull request #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#discussion_r374791835
##########
File path: scripts/get_all_heron_paths.sh
##########
@@ -112,7 +112,7 @@ function collect_generated_paths() {
# uniq to avoid doing blaze query on duplicates.
for path in $(find bazel-genfiles/ -name "*.java" | sed 's|/\{0,1\}bazel-genfiles/\{1,2\}|//|' | uniq); do
source_path=$(echo ${path} | sed 's|//|bazel-genfiles/|' | sed 's|/com/.*$||')
- echo "$(get_containing_library ${path}):${source_path}"
+# echo "$(get_containing_library ${path}):${source_path}"
Review comment:
It looks like this is the output of the for loop, which calls the removed functions. I don't really know this part of the code base but it might be useful I feel.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] nwangtw commented on issue #3444: Fixing setup
script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
nwangtw commented on issue #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444#issuecomment-582288131
Interesting. "//heron/spi/src/java:heron-spi-javadoc.zip.source/org/apache/heron/spi/statemgr/IStateManager.java" doesn't look like a valid target. "//heron/spi/src/java:heron-spi-javadoc.zip" is valid though. It looks like the ".source/org/..." part is somehow appended to the target name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-heron] joshfischer1108 closed pull request #3444: Fixing
setup script: Do Not Merge
Posted by GitBox <gi...@apache.org>.
joshfischer1108 closed pull request #3444: Fixing setup script: Do Not Merge
URL: https://github.com/apache/incubator-heron/pull/3444
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services