Skip to content

Commit

Permalink
Change how we embed docsite links in help text. (pantsbuild#12261)
Browse files Browse the repository at this point in the history
The issue is that we needed to generate URLs that work when displayed in
help messages on the console, but that also cross-link correctly when those same
help messages are used to generate docsite reference pages.

Previously we used raw links with the doc version embedded in the URL (e.g.,
https://www.pantsbuild.org/v2.6/docs/protobuf). And in many cases we had
to enclose that URL in parentheses to avoid issues with readme.com's linkifier
(such as it including a trailing period in the URL).

However it turns out that readme.com now modifies such URLs in markdown
to add the version. So https://www.pantsbuild.org/v2.6/docs/protobuf turns into
https://www.pantsbuild.org/v2.6/v2.6/docs/protobuf which is a bad link.

This change makes all this much more robust by moving the logic from help string
authoring time to docsite page generation time. When authoring help strings you simply
write URLs (either raw or, preferably, using the doc_url() util function). The code that
renders docsite pages detects docsite URLs in strings and rewrites them to proper
intra-doc markdown ([title](doc:slug)). It does so by fetching the titles from the live
pages.

This does assume that any links in generated docs are to files that already exist on the docsite.
But this is true today and is likely to continue to be true (since we have to know the page slug anyway).

[ci skip-build-wheels]
  • Loading branch information
benjyw committed Jul 2, 2021
1 parent 0182de9 commit 5aaa202
Show file tree
Hide file tree
Showing 25 changed files with 268 additions and 84 deletions.
52 changes: 48 additions & 4 deletions build-support/bin/generate_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,28 @@
from common import die

from pants.help.help_info_extracter import to_help_str
from pants.util.docutil import DocUrlMatcher, DocUrlRewriter, get_titles
from pants.version import MAJOR_MINOR

logger = logging.getLogger(__name__)


def main() -> None:
logging.basicConfig(format="[%(levelname)s]: %(message)s", level=logging.INFO)
version = determine_pants_version()
args = create_parser().parse_args()
version = determine_pants_version()
help_info = run_pants_help_all()
generator = ReferenceGenerator(args, version, help_info)
doc_urls = DocUrlMatcher().find_doc_urls(value_strs_iter(help_info))
logger.info("Found the following docsite URLs:")
for url in sorted(doc_urls):
logger.info(f" {url}")
logger.info("Fetching titles...")
slug_to_title = get_titles(doc_urls)
logger.info("Found the following titles:")
for slug, title in sorted(slug_to_title.items()):
logger.info(f" {slug}: {title}")
rewritten_help_info = rewrite_value_strs(help_info, slug_to_title)
generator = ReferenceGenerator(args, version, rewritten_help_info)
if args.sync:
generator.sync()
else:
Expand Down Expand Up @@ -142,8 +153,41 @@ def run_pants_help_all() -> Dict:
return cast(Dict, json.loads(run.stdout))


def value_strs_iter(help_info: dict) -> Iterable[str]:
def _recurse(val: Any) -> Iterable[str]:
if isinstance(val, str):
yield val
if isinstance(val, dict):
for v in val.values():
for x in _recurse(v):
yield x
if isinstance(val, list):
for v in val:
for x in _recurse(v):
yield x

for x in _recurse(help_info):
yield x


def rewrite_value_strs(help_info: dict, slug_to_title: dict[str, str]) -> dict:
"""Return a copy of the argument with rewritten docsite URLs."""
rewriter = DocUrlRewriter(slug_to_title)

def _recurse(val: Any) -> Any:
if isinstance(val, str):
return rewriter.rewrite(val)
if isinstance(val, dict):
return {k: _recurse(v) for k, v in val.items()}
if isinstance(val, list):
return [_recurse(x) for x in val]
return val

return cast(dict, _recurse(help_info))


class ReferenceGenerator:
def __init__(self, args: argparse.Namespace, version: str, help_info: Dict) -> None:
def __init__(self, args: argparse.Namespace, version: str, help_info: dict) -> None:
self._args = args
self._version = version

Expand Down Expand Up @@ -177,7 +221,7 @@ def _link(scope: str, *, sync: bool) -> str:
return f"reference-{url_safe_scope}" if sync else f"{url_safe_scope}.md"

@classmethod
def process_options_input(cls, help_info: Dict, *, sync: bool) -> Dict:
def process_options_input(cls, help_info: dict, *, sync: bool) -> Dict:
scope_to_help_info = help_info["scope_to_help_info"]

# Process the list of consumed_scopes into a comma-separated list, and add it to the option
Expand Down
13 changes: 12 additions & 1 deletion build-support/bin/generate_docs_test.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from generate_docs import html_safe, markdown_safe
from generate_docs import html_safe, markdown_safe, value_strs_iter


def test_markdown_safe():
Expand All @@ -10,3 +10,14 @@ def test_markdown_safe():

def test_html_safe():
assert "foo <code>bar==&#x27;baz&#x27;</code> qux" == html_safe("foo `bar=='baz'` qux")


def test_gather_value_strs():
help_info = {
"a": "foo",
"b": ["bar", 5, "baz"],
"c": 42,
"d": True,
"e": {"f": 5, "g": "qux", "h": {"i": "quux"}},
}
assert set(value_strs_iter(help_info)) == {"foo", "bar", "baz", "qux", "quux"}
4 changes: 2 additions & 2 deletions src/python/pants/backend/awslambda/python/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
targets_with_sources_types,
)
from pants.engine.unions import UnionMembership, UnionRule
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url
from pants.util.logging import LogLevel

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -119,7 +119,7 @@ async def package_python_awslambda(
"files targets, but Pants will not include them in the built Lambda. Filesystem APIs "
"like `open()` are not able to load files within the binary itself; instead, they "
"read from the current working directory."
f"\n\nInstead, use `resources` targets. See {bracketed_docs_url('resources')}."
f"\n\nInstead, use `resources` targets. See {doc_url('resources')}."
f"\n\nFiles targets dependencies: {files_addresses}"
)

Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/backend/awslambda/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from pants.engine.unions import UnionRule
from pants.source.filespec import Filespec
from pants.source.source_root import SourceRoot, SourceRootRequest
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url


class DeprecatedAwsLambdaInterpreterConstraints(InterpreterConstraintsField):
Expand Down Expand Up @@ -205,8 +205,8 @@ class PythonAWSLambda(Target):
PythonAwsLambdaRuntime,
)
help = (
"A self-contained Python function suitable for uploading to AWS Lambda.\n\nSee "
f"{bracketed_docs_url('awslambda-python')}."
"A self-contained Python function suitable for uploading to AWS Lambda.\n\n"
f"See {doc_url('awslambda-python')}."
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@
from pants.engine.unions import UnionRule
from pants.option.custom_types import target_option
from pants.option.subsystem import Subsystem
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url


class PythonProtobufSubsystem(Subsystem):
options_scope = "python-protobuf"
help = (
f"Options related to the Protobuf Python backend.\n\nSee {bracketed_docs_url('protobuf')}."
)
help = f"Options related to the Protobuf Python backend.\n\nSee {doc_url('protobuf')}."

@classmethod
def register_options(cls, register):
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/backend/codegen/protobuf/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.engine.target import COMMON_TARGET_FIELDS, BoolField, Dependencies, Sources, Target
from pants.util.docutil import bracketed_docs_url


# NB: We subclass Dependencies so that specific backends can add dependency injection rules to
# Protobuf targets.
from pants.util.docutil import doc_url


class ProtobufDependencies(Dependencies):
pass

Expand All @@ -25,4 +26,4 @@ class ProtobufGrpcToggle(BoolField):
class ProtobufLibrary(Target):
alias = "protobuf_library"
core_fields = (*COMMON_TARGET_FIELDS, ProtobufDependencies, ProtobufSources, ProtobufGrpcToggle)
help = f"Protobuf files used to generate various languages.\n\nSee {bracketed_docs_url('protobuf')}."
help = f"Protobuf files used to generate various languages.\n\nSee f{doc_url('protobuf')}."
6 changes: 3 additions & 3 deletions src/python/pants/backend/python/goals/package_pex_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
targets_with_sources_types,
)
from pants.engine.unions import UnionMembership, UnionRule
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url
from pants.util.logging import LogLevel

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -120,8 +120,8 @@ async def package_pex_binary(
"targets, but Pants will not include them in the PEX. Filesystem APIs like `open()` "
"are not able to load files within the binary itself; instead, they read from the "
"current working directory."
"\n\nInstead, use `resources` targets or wrap this `pex_binary` in an `archive`. See "
f"{bracketed_docs_url('resources')}."
"\n\nInstead, use `resources` targets or wrap this `pex_binary` in an `archive`. "
f"See {doc_url('resources')}."
f"\n\nFiles targets dependencies: {files_addresses}"
)

Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/goals/setup_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
from pants.engine.unions import UnionMembership, UnionRule, union
from pants.option.subsystem import Subsystem
from pants.python.python_setup import PythonSetup
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url
from pants.util.logging import LogLevel
from pants.util.memo import memoized_property
from pants.util.meta import frozen_after_init
Expand All @@ -88,7 +88,7 @@ class OwnershipError(Exception):

def __init__(self, msg: str):
super().__init__(
f"{msg} See {bracketed_docs_url('python-distributions')} for "
f"{msg} See {doc_url('python-distributions')} for "
f"how python_library targets are mapped to distributions."
)

Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/lint/pylint/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from pants.core.util_rules.config_files import ConfigFilesRequest
from pants.engine.addresses import UnparsedAddressInputs
from pants.option.custom_types import file_option, shell_str, target_option
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url


class Pylint(PythonToolBase):
Expand Down Expand Up @@ -74,7 +74,7 @@ def register_options(cls, register):
"example, if your plugin is at `build-support/pylint/custom_plugin.py`, add "
"'build-support/pylint' to `[source].root_patterns` in `pants.toml`. This is "
"necessary for Pants to know how to tell Pylint to discover your plugin. See "
f"{bracketed_docs_url('source-roots')}\n\nYou must also set `load-plugins=$module_name` in "
f"{doc_url('source-roots')}\n\nYou must also set `load-plugins=$module_name` in "
"your Pylint config file, and set the `[pylint].config` option in `pants.toml`."
"\n\nWhile your plugin's code can depend on other first-party code and third-party "
"requirements, all first-party dependencies of the plugin must live in the same "
Expand Down
20 changes: 10 additions & 10 deletions src/python/pants/backend/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
from pants.option.subsystem import Subsystem
from pants.python.python_setup import PythonSetup
from pants.source.filespec import Filespec
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url
from pants.util.frozendict import FrozenDict

logger = logging.getLogger(__name__)
Expand All @@ -69,9 +69,9 @@ class InterpreterConstraintsField(StringSequenceField):
"`CPython` as a shorthand, e.g. '>=2.7' will be expanded to 'CPython>=2.7'.\n\nSpecify "
"more than one element to OR the constraints, e.g. `['PyPy==3.7.*', 'CPython==3.7.*']` "
"means either PyPy 3.7 _or_ CPython 3.7.\n\nIf the field is not set, it will default to "
"the option `[python-setup].interpreter_constraints`.\n\nSee "
f"{bracketed_docs_url('python-interpreter-compatibility')} for how these interpreter "
f"constraints are merged with the constraints of dependencies."
"the option `[python-setup].interpreter_constraints`.\n\n"
f"See {doc_url('python-interpreter-compatibility')} for how these interpreter "
"constraints are merged with the constraints of dependencies."
)

def value_or_global_default(self, python_setup: PythonSetup) -> Tuple[str, ...]:
Expand Down Expand Up @@ -390,7 +390,7 @@ class PexBinary(Target):
help = (
"A Python target that can be converted into an executable PEX file.\n\nPEX files are "
"self-contained executable files that contain a complete Python environment capable of "
f"running the target. For more information, see {bracketed_docs_url('pex-files')}."
f"running the target. For more information, see {doc_url('pex-files')}."
)


Expand Down Expand Up @@ -471,7 +471,7 @@ class PythonTests(Target):
help = (
"Python tests, written in either Pytest style or unittest style.\n\nAll test util code, "
"other than `conftest.py`, should go into a dedicated `python_library()` target and then "
f"be included in the `dependencies` field.\n\nSee {bracketed_docs_url('python-test-goal')}."
f"be included in the `dependencies` field.\n\nSee {doc_url('python-test-goal')}."
)


Expand Down Expand Up @@ -650,7 +650,7 @@ class PythonRequirementLibrary(Target):
"Python requirements inline in a BUILD file. If you have a `requirements.txt` file "
"already, you can instead use the macro `python_requirements()` to convert each "
"requirement into a `python_requirement_library()` target automatically.\n\nSee "
f"{bracketed_docs_url('python-third-party-dependencies')}."
f"{doc_url('python-third-party-dependencies')}."
)


Expand Down Expand Up @@ -710,8 +710,8 @@ class PythonProvidesField(ScalarField, ProvidesField):
"`name`. You can also set almost any keyword argument accepted by setup.py in the "
"`setup()` function: "
"(https://packaging.python.org/guides/distributing-packages-using-setuptools/#setup-args)."
f"\n\nSee {bracketed_docs_url('plugins-setup-py')} for how to write a plugin to "
f"dynamically generate kwargs."
f"\n\nSee {doc_url('plugins-setup-py')} for how to write a plugin to "
"dynamically generate kwargs."
)

@classmethod
Expand Down Expand Up @@ -742,5 +742,5 @@ class PythonDistribution(Target):
)
help = (
"A publishable Python setuptools distribution (e.g. an sdist or wheel).\n\nSee "
f"{bracketed_docs_url('python-distributions')}."
f"{doc_url('python-distributions')}."
)
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/typecheck/mypy/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from pants.engine.target import FieldSet, Target, TransitiveTargets, TransitiveTargetsRequest
from pants.engine.unions import UnionRule
from pants.python.python_setup import PythonSetup
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url
from pants.util.logging import LogLevel
from pants.util.ordered_set import FrozenOrderedSet, OrderedSet
from pants.util.strutil import pluralize
Expand Down Expand Up @@ -99,7 +99,7 @@ def check_and_warn_if_python_version_configured(
logger.warning(
f"You set {formatted_configured}. Normally, Pants would automatically set this for you "
"based on your code's interpreter constraints "
f"({bracketed_docs_url('python-interpreter-compatibility')}). Instead, it will "
f"({doc_url('python-interpreter-compatibility')}). Instead, it will "
"use what you set.\n\n(Automatically setting the option allows Pants to partition your "
"targets by their constraints, so that, for example, you can run MyPy on Python 2-only "
"code and Python 3-only code at the same time. This feature may no longer work.)"
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/core/goals/tailor.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
UnexpandedTargets,
)
from pants.engine.unions import UnionMembership, union
from pants.util.docutil import doc_url
from pants.util.frozendict import FrozenDict
from pants.util.logging import LogLevel
from pants.util.memo import memoized
Expand Down Expand Up @@ -227,7 +228,7 @@ def register_options(cls, register):
type=dict,
help="A mapping from standard target type to custom type to use instead. The custom "
"type can be a custom target type or a macro that offers compatible functionality "
"to the one it replaces (see https://www.pantsbuild.org/docs/macros).",
f"to the one it replaces (see {doc_url('macros')}).",
)

@property
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/engine/internals/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
from pants.engine.unions import UnionMembership
from pants.option.global_options import GlobalOptions, OwnersNotFoundBehavior
from pants.source.filespec import matches_filespec
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url
from pants.util.logging import LogLevel
from pants.util.ordered_set import FrozenOrderedSet, OrderedSet

Expand Down Expand Up @@ -418,9 +418,9 @@ def _log_or_raise_unmatched_owners(
f"target whose `sources` field includes the file."
)
msg = (
f"{prefix} See {bracketed_docs_url('targets')} for more information on target definitions."
f"{prefix} See {doc_url('targets')} for more information on target definitions."
f"\n\nYou may want to run `./pants tailor` to autogenerate your BUILD files. See "
f"{bracketed_docs_url('create-initial-build-files')}.{option_msg}"
f"{doc_url('create-initial-build-files')}.{option_msg}"
)

if owners_not_found_behavior == OwnersNotFoundBehavior.warn:
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/engine/internals/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from pants.base.parse_context import ParseContext
from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.engine.internals.target_adaptor import TargetAdaptor
from pants.util.docutil import bracketed_docs_url
from pants.util.docutil import doc_url
from pants.util.frozendict import FrozenDict


Expand Down Expand Up @@ -105,7 +105,7 @@ def parse(
original = e.args[0].capitalize()
help_str = (
"If you expect to see more symbols activated in the below list,"
f" refer to {bracketed_docs_url('enabling-backends')} for all available"
f" refer to {doc_url('enabling-backends')} for all available"
" backends to activate."
)

Expand Down Expand Up @@ -142,6 +142,6 @@ def error_on_imports(build_file_content: str, filepath: str) -> None:
raise ParseError(
f"Import used in {filepath} at line {lineno}. Import statements are banned in "
"BUILD files because they can easily break Pants caching and lead to stale results. "
f"\n\nInstead, consider writing a macro ({bracketed_docs_url('macros')}) or "
f"writing a plugin ({bracketed_docs_url('plugins-overview')}."
f"\n\nInstead, consider writing a macro ({doc_url('macros')}) or "
f"writing a plugin ({doc_url('plugins-overview')}."
)
Loading

0 comments on commit 5aaa202

Please sign in to comment.