Document usage of BFG and modify usage to be more ergonomic by petroseskinder · Pull Request #24 · bazelbuild/BUILD_file_generator

petroseskinder · 2017-10-16T08:52:10Z

Bouncing off #21, the following changes arose after I tried (and failed) to use Bfg on a simple java project.

Documentation

NOTE: This is a WIP. My primary intent is to get the ball rolling on documentation.

It has been roughly a year since I last made serious contributions to Bfg, so there were a number of basic facts I had to recollect (e.g. like motivation, usage, etc). I augmented the README with whatever I felt was inadequately documented. Admittedly, there are a number of TODO statements littered through the README for things that were still unclear to me. For example, I don't really understand how you what the new proto format should look like.

Ergonomics

One thing I found incredibly annoying was needing to perform

bazel run //src/main/java/com/google/devtools/build/bfg:bfg

I found it more ergonomic to perform

bazel run //src:bfg

Consequently, I have moved the java_binary targets into their respective top level directories lang/ and src/. There is little need for the java_binary's to be wedged deep into the code base.

@cgrushko, undoubtedly, I have likely made stylistic snafus with how I managed visibility. Please let me know what the best approach is there.

…tion to lang/

greggdonovan · 2017-10-16T21:11:37Z

README.md

+1. Language specific parsers (LSP) 
+2. The BFG binary
+
+The LSPs read your source code and generate a class dependency graph in the form of a protobuff. To generate your BUILD files, you pass the generated protobuff into the BFG binary. 


nit: protobuff looks like a typo

greggdonovan · 2017-10-16T21:14:25Z

README.md

+
+### Step 0: Installation Instructions
+
+To install BFG...TODO


Do we have an eventual goal for users will download and install BFG? Would it go in bazel-dev?

As far as I know, we have yet to define that goal.

I think it would be nice if users didn't have to install BFG at all, and they could something like

bazel generate <options-and-configs> project/

What do other projects do on this front? I am not the hugest fan of needing to go get buildozer.

greggdonovan · 2017-10-16T21:16:26Z

lang/java/src/main/java/com/google/devtools/build/bfg/JavaSourceFileParserCli.java

- * dot file representing the class level dependencies between these source files.
+ * Entry point to the BFG java source file parser. Given a list of source files, 
+ * it obtains the class and file level dependencies between the files, generates a
+ * dependency graph, and outputs this graph as a protobuff.


s/protobuff/protobuf

petroseskinder

@cgrushko, I added comments where I desired your advice.

petroseskinder · 2017-10-17T19:12:52Z

lang/README.md

+
+```
+
+TODO(bazel-devel): what do the command line options mean??? There is a TODO in the Cli asking how the files will be obtained.


@cgrushko similar question as before. How do you actually run the JavaSourceFileParserCli?

See above. I'm not sure what the TODO means (we should figure out, or delete it :)

petroseskinder · 2017-10-17T19:13:36Z

README.md

+TODO(bazel-devel): add explanation and valid example arguments.
+
+```bash
+bazel run //lang:JavaSourceFileParserCli <TODO>


@cgrushko how do you actually run the JavaSourceFileParserCli? For example what is the command line args for for google-java-format?

I wasn't able to immediately figure out what the command line format was, or how the input proto should be formatted.

For google-java-format it should probably be

bazel run //lang/java/src/main/java/com/google/devtools/build/bfg:JavaSourceFileParserCli --roots=core/src/main/java,core/src/test/java $(find core/src/main/java/ core/src/test/java/ -name \*.java) > bfg.bin

It's a bit silly that we have to pass both roots and files, but it comes from Google where you don't want to read the entire java tree just because it's the root. We can perhaps improve this by scanning all of roots if the user doesn't provide source files, but it's for another day.

It should print usage information if run without flags (but I haven't tried it lately). Does it now?
By format, what do you mean?

petroseskinder · 2017-10-17T19:14:07Z

README.md

+
+### Step 0: Installation Instructions
+
+To install BFG...TODO


@cgrushko shall I open an issue for "how to install bfg"?

Is it much more than cloning the repo and building two targets?
Perhaps build the *_deploy.jar targets so the output can be copied around.

petroseskinder · 2017-10-17T19:18:36Z

lang/java/src/main/java/com/google/devtools/build/bfg/BUILD

        "//thirdparty/jvm/com/google/guava",
        "//thirdparty/jvm/org/eclipse/jdt:org_eclipse_jdt_core",
    ],
+    visibility = ["//lang:__subpackages__"],


@cgrushko is this the appropriate visibility for the JavaSourceFileParser?

For context, I moved the JavaSourceFileParserCli binary to the top of the directory. This was so one simply needed to type

//src:JavaSourceFileParserCli

rather than the more tedious

//src/main/java/com/google/devtools/build/bfg:JavaSourceFileParserCli

The previous approach was annoyingly tedious.

As far as the visibility, I want to expose JavaSourceFileParser to just JavaSourceFileParserCli and its subpackages. However, that introduces a circular dependencies.

Yeah, you can't set visibility to a single rule.

How often do you expect users to build the thing?
When I built it it was from an IDE so didn't have to type it at all.
To run it, I had a symlink, but this arguably raises the bar for contributions.

We can add an alias() to lang/java/BUILD that points at the full path, but tbh this seems like dead weight.

petroseskinder · 2017-10-17T19:21:16Z

src/main/java/com/google/devtools/build/bfg/BUILD

-    main_class = "com.google.devtools.build.bfg.Bfg",
-    stamp = 1,
-    runtime_deps = [":Bfg"],
+    visibility = [


@cgrushko similar visibility questions.

petroseskinder · 2017-11-08T00:58:51Z

@spomorski since carmi is on paternity leave. Here is the documentation I was referring to.

sergetron

Approval for documentation.

sergetron

Approval for documentation.

sergetron

Approval for documentation.

sergetron · 2017-12-04T18:50:07Z

I don't have write access to this repository, but I provided approval from the documentation standpoint.

cgrushko · 2017-12-04T22:17:16Z

README.md

+# BUILD File Generator (BFG)

-BUILD File Generator generates Bazel BUILD files for Java code.
+BUILD File Generator, or BFG for short, is a tool for generating Bazel BUILD files from one's source code. Instead of manually going through your project and creating BUILD files and their appropriate build rules, you can rely on BFG to do it for you! 


I feel more is less here :) does it really contribute? as an engineer I personally prefer as little prose as possible :)

In this respect, laying out in bullets what the project does (even if it simplifies things) makes it easier for me to quickly scan the readme and see if the project is right for me.

cgrushko · 2017-12-04T22:26:02Z

README.md

+
+### Why is it useful?
+
+Bazel promises fast and correct builds, _especially_ for incremental builds. Rather than rebuild your entire codebase, Bazel _only_ rebuilds what is necessary, the targets that you have changed. For the remainder of your code, it relies on cached versions. 


I'm super late to the party, so feel free to reject.
We also have different writing styles (and I'm not a native English speaker anyway) so there's probably no "better" approach here.

I suggest the following as a replacement for this section -

"""
Having all sources in a single BUILD rule doesn't allow Bazel to parallelize and cache builds. In order to fully benefit from Bazel, one must write multiple BUILD rules and connect them.

This project automates writing granular BUILD rules that allow Bazel to parallelize and cache builds.

It's useful to quickly try out Bazel on your project, as well as to periodically optimize your build graph.
"""

cgrushko · 2017-12-04T22:27:07Z

README.md

-TODO
+BFG is composed of two general components
+
+1. Language specific parsers (LSP)


perhaps "parser" instead of "LSP" thorough the doc?

cgrushko · 2017-12-04T22:27:41Z

README.md

+
+### Step 0: Installation Instructions
+
+To install BFG...TODO


Is it much more than cloning the repo and building two targets?
Perhaps build the *_deploy.jar targets so the output can be copied around.

cgrushko · 2017-12-04T22:29:19Z

README.md

+```
+
+This generates a protobuf at TODO.
+TODO(bazel-devel): add clear protocol for how the protos are structured. Useful for other developers who may want to write LSPs.


we can link to the proto definition,

BUILD_file_generator/src/main/java/com/google/devtools/build/bfg/bfg.proto

Line 9 in 672c557

message ParserOutput {

cgrushko · 2017-12-04T22:41:28Z

lang/README.md

+
+```bash
+
+bazel run //lang:JavaSourceFileParserCli 


do you prefer not using //lang/java/src/main/java/com/google/devtools/build/bfg:JavaSourceFileParserCli ?

cgrushko · 2017-12-04T22:41:35Z

lang/README.md

+### Usage
+
+```bash
+


superfluous newlines?

cgrushko · 2017-12-04T22:42:28Z

lang/README.md

+
+```
+
+TODO(bazel-devel): what do the command line options mean??? There is a TODO in the Cli asking how the files will be obtained.


See above. I'm not sure what the TODO means (we should figure out, or delete it :)

cgrushko · 2017-12-04T22:45:12Z

lang/java/src/main/java/com/google/devtools/build/bfg/BUILD

        "//thirdparty/jvm/com/google/guava",
        "//thirdparty/jvm/org/eclipse/jdt:org_eclipse_jdt_core",
    ],
+    visibility = ["//lang:__subpackages__"],


Yeah, you can't set visibility to a single rule.

How often do you expect users to build the thing?
When I built it it was from an IDE so didn't have to type it at all.
To run it, I had a symlink, but this arguably raises the bar for contributions.

We can add an alias() to lang/java/BUILD that points at the full path, but tbh this seems like dead weight.

cgrushko · 2017-12-04T22:45:31Z

lang/java/src/main/java/com/google/devtools/build/bfg/JavaSourceFileParserCli.java

 /**
- * Entry point to BFG java source file parser. Given a list of source files, it prints a graph viz
- * dot file representing the class level dependencies between these source files.
+ * Entry point to the BFG java source file parser. Given a list of source files, 


let's leave this to another PR?
same for other code changes.

sergetron · 2017-12-08T16:24:28Z

Hey guys, any movement on this? I'm writing official documentation for bazel-deps and Bfg and would love to use this readme as source material. Thank you!

petroseskinder added 3 commits October 16, 2017 00:33

Makes ergonomic changes to JavaSourceFileParserCli and adds documenta…

659ee55

…tion to lang/

Makes running Bfg java_binary target more ergonomic

c9979ed

Creates skeleton of the project documentation

c31bbae

petroseskinder requested review from cgrushko and greggdonovan October 16, 2017 08:52

greggdonovan reviewed Oct 16, 2017

View reviewed changes

petroseskinder and others added 2 commits October 16, 2017 19:42

Fix typo and remove trailing spaces.

9bacb72

Merge branch 'master' into review_mvfiles

db74785

petroseskinder commented Oct 17, 2017

View reviewed changes

sergetron approved these changes Dec 4, 2017

View reviewed changes

Merge branch 'master' into review_mvfiles

2fba201

cgrushko reviewed Dec 4, 2017

View reviewed changes

cgrushko mentioned this pull request Dec 19, 2017

Add motivation and rudimentary running instructions to README.md #34

Merged


		```

		TODO(bazel-devel): what do the command line options mean??? There is a TODO in the Cli asking how the files will be obtained.


		### Why is it useful?

		Bazel promises fast and correct builds, _especially_ for incremental builds. Rather than rebuild your entire codebase, Bazel _only_ rebuilds what is necessary, the targets that you have changed. For the remainder of your code, it relies on cached versions.

Conversation

petroseskinder commented Oct 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation

Ergonomics

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petroseskinder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petroseskinder commented Nov 8, 2017

Uh oh!

sergetron left a comment

Choose a reason for hiding this comment

Uh oh!

sergetron left a comment

Choose a reason for hiding this comment

Uh oh!

sergetron left a comment

Choose a reason for hiding this comment

Uh oh!

sergetron commented Dec 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergetron commented Dec 8, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

petroseskinder commented Oct 16, 2017 •

edited

Loading