From d9b3e3b743f00bf00ba8d1306c56a477ec50fc4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Csomor?= Date: Mon, 24 Sep 2018 14:45:32 +0200 Subject: [PATCH] Fix runfile lookup in skydoc/main.py (#108) * Fix runfile lookup in skydoc/main.py `skydoc/main.py` now uses Bazel's built-in runfiles library to look up data-dependencies. This commit introduces one hack: we use os.path.dirname (actually: posixpath.dirname) on a path returned by the runfiles library. This is wrong, because the runfiles path does not necessarily correlate with the actual path of the file, i.e. the path that the runfiles-lookup library returns. For example, if //foo:a and //foo:generated-b are both data-dependencies, they'll both be under the "foo" runfiles-directory, but will be in different real paths. This is fine on Linux because Bazel generates a runfiles tree with symlinks, but breaks on Windows because there Bazel only generates a runfiles manifest. Fortunately, for now, all files under `skydoc/templates` are source files, so this hack works. If anyone ever adds generated files to //skydoc/templates:templates, this hack will break. See https://github.com/bazelbuild/skydoc/issues/58 --- skydoc/BUILD | 1 + skydoc/main.py | 84 +++++++++++++++++++++++--------------------------- 2 files changed, 40 insertions(+), 45 deletions(-) diff --git a/skydoc/BUILD b/skydoc/BUILD index 1ee1085..1293320 100644 --- a/skydoc/BUILD +++ b/skydoc/BUILD @@ -96,5 +96,6 @@ py_binary( ":rule_extractor", "//external:gflags", "//external:jinja2", + "@bazel_tools//tools/python/runfiles", ], ) diff --git a/skydoc/main.py b/skydoc/main.py index 7fd5d5e..8185ef5 100755 --- a/skydoc/main.py +++ b/skydoc/main.py @@ -19,12 +19,14 @@ import jinja2 import mistune import os +import posixpath import re import shutil import sys import tempfile import zipfile +from bazel_tools.tools.python.runfiles import runfiles as runfiles_lib from skydoc import common from skydoc import load_extractor from skydoc import macro_extractor @@ -68,9 +70,13 @@ CSS_FILE = 'main.css' -def _create_jinja_environment(site_root, link_ext): +def _create_jinja_environment(runfiles, site_root, link_ext): + def _Load(path): + return runfiles.Rlocation(posixpath.join(WORKSPACE_DIR, TEMPLATE_PATH, + path)) + env = jinja2.Environment( - loader=jinja2.FileSystemLoader(_runfile_path(TEMPLATE_PATH)), + loader=jinja2.FunctionLoader(_Load), keep_trailing_newline=True, line_statement_prefix='%') env.filters['markdown'] = lambda text: jinja2.Markup(mistune.markdown(text)) @@ -82,40 +88,8 @@ def _create_jinja_environment(site_root, link_ext): # TODO(dzc): Remove this workaround once we switch to a self-contained Python # binary format such as PEX. -def _runfile_path(path): - """Prepends the given path with the path to the root of the runfiles tree. - - The files that skydoc depend on are generated in the Bazel runfiles tree. - There is no built-in way to get the root of the runfiles tree but it is - possible to generate it from sys.argv[0]. The code here is adapted from the - Bazel Python launcher stub. - - Args: - path: The relative path from the root of the runfiles tree. - - Returns: - Returns path prepended with the absolute path to the root of the runfiles - tree. - """ - script_filename = os.path.abspath(sys.argv[0]) - while True: - runfiles_dir = script_filename + '.runfiles' - if os.path.isdir(runfiles_dir): - break - - # Follow a symlink and try again. - if os.path.islink(script_filename): - link = os.readlink(script_filename) - script_filename = os.path.join(os.path.dirname(script_filename), link) - continue - - matchobj = re.match("(.*\.runfiles)/.*", os.path.abspath(sys.argv[0])) - if matchobj: - runfiles_dir = matchobj.group(1) - break - - raise AssertionError('Cannot find .runfiles directory.') - return os.path.join(runfiles_dir, WORKSPACE_DIR, path) +def _runfile_path(runfiles, path): + return runfiles.Rlocation(posixpath.join(WORKSPACE_DIR, path)) def merge_languages(macro_language, rule_language): for rule in rule_language.rule: @@ -140,9 +114,10 @@ def __init__(self, output_dir, output_file, output_zip, overview, class MarkdownWriter(object): """Writer for generating documentation in Markdown.""" - def __init__(self, writer_options): + def __init__(self, writer_options, runfiles): self.__options = writer_options - self.__env = _create_jinja_environment(self.__options.site_root, + self.__env = _create_jinja_environment(runfiles, + self.__options.site_root, self.__options.link_ext) def write(self, rulesets): @@ -205,9 +180,11 @@ def _write_overview(self, output_dir, rulesets): class HtmlWriter(object): """Writer for generating documentation in HTML.""" - def __init__(self, options): + def __init__(self, options, runfiles): + self.__runfiles = runfiles self.__options = options - self.__env = _create_jinja_environment(self.__options.site_root, + self.__env = _create_jinja_environment(runfiles, + self.__options.site_root, self.__options.link_ext) def write(self, rulesets): @@ -231,8 +208,9 @@ def write(self, rulesets): with zipfile.ZipFile(self.__options.output_file, 'w') as zf: for output_file, output_path in output_files: zf.write(output_file, output_path) - zf.write(os.path.join(_runfile_path(CSS_PATH), CSS_FILE), - '%s' % CSS_FILE) + zf.write(_runfile_path(self.__runfiles, + posixpath.join(CSS_PATH, CSS_FILE)), + '%s' % CSS_FILE) else: for output_file, output_path in output_files: dest_file = os.path.join(self.__options.output_dir, output_path) @@ -242,7 +220,8 @@ def write(self, rulesets): shutil.copyfile(output_file, dest_file) # Copy CSS file. - shutil.copyfile(os.path.join(_runfile_path(CSS_PATH), CSS_FILE), + shutil.copyfile(_runfile_path(self.__runfiles, + posixpath.join(CSS_PATH, CSS_FILE)), os.path.join(self.__options.output_dir, CSS_FILE)) finally: # Delete temporary directory. @@ -290,6 +269,21 @@ def main(argv): print(err.message) sys.exit(1) + runfiles = runfiles_lib.Create() + if not runfiles: + # TODO(laszlocsomor): fix https://github.com/bazelbuild/bazel/issues/6212 + # and remove this if-block once Bazel is released with that bugfix. + if (not os.environ.get("RUNFILES_DIR") and + not os.environ.get("RUNFILES_MANIFEST_FILE")): + argv0 = sys.argv[0] + pos = argv0.rfind('%s%s%s' % (os.sep, WORKSPACE_DIR, os.sep)) + if pos > -1: + runfiles = runfiles_lib.CreateDirectoryBased(argv0[0:pos - 1]) + + if not runfiles: + print("Cannot load runfiles") + sys.exit(1) + rulesets = [] load_sym_extractor = load_extractor.LoadExtractor() for bzl_file in bzl_files: @@ -316,10 +310,10 @@ def main(argv): FLAGS.output_dir, FLAGS.output_file, FLAGS.zip, FLAGS.overview, FLAGS.overview_filename, FLAGS.link_ext, FLAGS.site_root) if FLAGS.format == "markdown": - markdown_writer = MarkdownWriter(writer_options) + markdown_writer = MarkdownWriter(writer_options, runfiles) markdown_writer.write(rulesets) elif FLAGS.format == "html": - html_writer = HtmlWriter(writer_options) + html_writer = HtmlWriter(writer_options, runfiles) html_writer.write(rulesets) else: sys.stderr.write(