Fix #2679: remove-exploration-java-proto-lite from the model library

[yashraj-01]

Explanation

Fixes #2679 . Removed :exploration_java_proto_lite from the model library. Added visibility = ["//visibility:public"], to the java_lite_proto_library that has the name exploration_java_proto_lite.

The build was successful after Step 1 and Step 2. So, it was not required to follow Step 3.

Checklist

• The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
• The PR follows the style guide.
• The PR does not contain any unnecessary auto-generated code from Android Studio.
• The PR is made from a branch that's not called "develop".
• The PR is made from a branch that is up-to-date with "develop".
• The PR's branch is based on "develop" and not on any other branch.
• The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

[yashraj-01]

@fsharpasharp @rt4914 PTAL.

[fsharpasharp]

It is unexpected to me that the app will build without step 3. Make sure that you are building the entire application.

We'll have to wait for the checks to be fixed and rerun the checks. Everything else looks good.

[vinitamurthi]

In general we should avoid doing this. Any reason why we want to make this public?

[fsharpasharp]

@vinitamurthi, are you saying that we should be explicit about the visibility?

[BenHenning]

Does #2699 (comment) help clarify the need @vinitamurthi or do you have a separate concern?

[yashraj-01]

@fsharpasharp I am somehow getting this error now. Previously it worked fine but now this error is coming. I also followed the fix provided above but it also does not work. Please help me to fix this error.

[fsharpasharp]

Replace ~ with your home directory.

[yashraj-01]

Replace ~ with your home directory.

In which of the following commands should I replace it. I'm currently at step 4.

[fsharpasharp]

In your ~/.bazelrc file

[BenHenning]

Similar to #2699 (comment) we need verify that all targets are now building with this change. Updating the branch to trigger a full run of all affected Bazel test targets.

[yashraj-01]

It is unexpected to me that the app will build without step 3. Make sure that you are building the entire application.

We'll have to wait for the checks to be fixed and rerun the checks. Everything else looks good.

@fsharpasharp I again ran the build and it is showing build successful. What do you suggest I should do?

[yashraj-01]

@BenHenning @vinitamurthi PTAL.

[fsharpasharp]

I see that there is one failing check. Try to identify the issue and make sure that the all the tests pass locally as well. Let me know if you get stuck.

Run

~/oppia-bazel/bazel test //app/...

and

~/oppia-bazel/bazel test //...

if the previous one passes.

[yashraj-01]

I see that there is one failing check. Try to identify the issue and make sure that the all the tests pass locally as well. Let me know if you get stuck.

Run

~/oppia-bazel/bazel test //app/...

and

~/oppia-bazel/bazel test //...

if the previous one passes.

#2722 (comment)

I asked @FareesHussain about this failed check and he said it just needs a re-run. I have attached the link of the comment.

Also, when I ran the test you mentioned, it gets stuck at check number 11,978.

Please, tell me how to proceed now.

[fsharpasharp]

Okay, could you try to sync to head and run the checks again? It could help us rule out if it's a problem with the current commit or with the CI.

[BenHenning]

Deferring to @fsharpasharp to figure out why we're seeing what appears to be Java proto lite libraries being generated for each proto (including transitive proto dependencies). Please reassign me once this PR is ready for review.

[BenHenning]

Ah, didn't meant to close the PR. Checks are re-running now, sorry about that. Old checks can be seen in the commit above, but some might get cancelled by the automatic canceller.

[yashraj-01]

@fsharpasharp @BenHenning all the checks have passed. PTAL. Thanks.

[fsharpasharp]

I'm going to spend some time to look into where the dependency is getting pulled from. I'll open some similar issues in the meantime, if you are itching to do some more work. Thanks for the patience, @yashraj-iitr.

[fsharpasharp]

  format_import_proto_library(                                                                                                                                                                 
      name = "question",                                                                                                                                                                       
      src = "src/main/proto/question.proto",                                                                                                                                                   
      deps = [                                                                                                                                                                                 
          ":exploration_proto",                                                                                                                                                                
          ":subtitled_html_proto",                                                                                                                                                             
          ":subtitled_unicode_proto",                                                                                                                                                          
      ],                                                                                                                                                                                       
  )                                                                                                                                                                                            
                                                                                                                                                                                               
  java_lite_proto_library(                                                                                                                                                                     
      name = "question_java_proto_lite",                                                                                                                                                       
      deps = [":question_proto"],                                                                                                                                                              
  )   

question_java_proto_lite is indirectly exporting the exploration proto. I think we can go ahead and merge this PR, @BenHenning . When we remove the question_java_proto_lite from the exports list of model we can add both dependencies back to where they are used.

To be thorough we could remove

visibility = ["//visibility:public"],

from the exploration_java_proto_lite library in this PR. This could make it clear how the dependency propagates.

[BenHenning]

Ack, it seems like we can't solve this right now but moving in this direction is still satisfying Bazel's proto best practices.

[BenHenning]

Per my comment above, we'll just keep in mind that protos imply transitive dependencies as we modularize the codebase. We should take care to try and depend on the most specific proto possible for a given context.

Approved since this is solving the original issue. Thanks @yashraj-iitr!