Bazel add html support #765
Conversation
| parser = argparse.ArgumentParser(add_help=False) | ||
| parser.add_argument('-o') | ||
| parser.add_argument('--oformat') | ||
| (options, _) = parser.parse_known_args(param_file_args) |
There was a problem hiding this comment.
Maybe options = parser.parse_known_args(param_file_args)[0] (or at least drop the braces on the left hand side of the expression.
|
|
||
| # The output file name is the name of the build rule that was built. | ||
| # Add an appropriate file extension based on --oformat. | ||
| if oformat is not None: |
There was a problem hiding this comment.
If oformat is None don't we want to default to "js" since that is what emcc will do?
How about parser.add_argument('--oformat', default='js') above and then you can remove this condition?
There was a problem hiding this comment.
This check is a bit overly cautious, but I prefer to keep it in there. If oformat is None, I'd rather emcc handle what to do by default, just in case emcc ever changes. Also, in the crosstool we're setting --oformat=js as a default, so realistically this won't ever be None.
| # Re-write response file if needed. | ||
| if replace_response_file: |
There was a problem hiding this comment.
(unrelated) I wonder if we can completely remove this at some point... how does bazel + clang deal with this issue?
There was a problem hiding this comment.
There's an internal P3 bug filed with the bazel folks that we're tracking.
| # the output_file so it has the proper extension. | ||
| # Note that if you do something like name your build rule "foo.js" and pass | ||
| # "--oformat=html", emscripten will write to the same file for both the js and | ||
| # html output, overwriting the js output entirely with the html. |
There was a problem hiding this comment.
This sounds like there is still a bug in emscripten?
There was a problem hiding this comment.
Possibly. Emscripten could ensure it's only writing to files it just created and throw an error. I can file a bug for later.
There was a problem hiding this comment.
Filed emscripten-core/emscripten#13729 for later
* allow the bazel toolchain to output html files * allow for cc_binary rule names to end in .js * fix python name * continue to call emcc instead of em++ for now * small cleanup Co-authored-by: Mitch Foley <[email protected]>
This gets us most of the way there for #759.