Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow building all docs with a local repo #659

Merged
merged 24 commits into from
Mar 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions build_docs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ from __future__ import print_function

import logging
from os import environ, getgid, getuid
from os.path import basename, dirname, exists, isdir, join, realpath
from os.path import basename, dirname, exists, expanduser, isdir
from os.path import join, realpath
import re
import subprocess
from sys import platform, version_info
Expand Down Expand Up @@ -117,8 +118,7 @@ def run_build_docs(args):
'-v',
'%s:%s:ro,cached' % (repo_root, repo_mount)
])
build_docs_args.append(
repo_mount + path.replace(repo_root, ''))
return repo_mount + path.replace(repo_root, '')

open_browser = False
args = Args(args)
Expand Down Expand Up @@ -164,7 +164,8 @@ def run_build_docs(args):
doc_file = realpath(args.next_arg_or_err())
if not exists(doc_file):
raise ArgError("Can't find --doc %s" % doc_file)
mount_docs_repo_and_dockerify_path(dirname(doc_file), doc_file)
build_docs_args.append(mount_docs_repo_and_dockerify_path(
dirname(doc_file), doc_file))
saw_doc = True
elif arg == '--open':
docker_args.extend(['--publish', '8000:8000/tcp'])
Expand Down Expand Up @@ -200,7 +201,19 @@ def run_build_docs(args):
resource_dir = realpath(args.next_arg_or_err())
if not isdir(resource_dir):
raise ArgError("Can't find --resource %s" % resource_dir)
mount_docs_repo_and_dockerify_path(resource_dir, resource_dir)
build_docs_args.append(mount_docs_repo_and_dockerify_path(
resource_dir, resource_dir))
elif arg == '--sub_dir':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you add features, all the manual parsing of argv is going to cost you more and more. argparse isn't great, but is there any value in considering it over full DIY?

(Normally, I would suggest Click, but I know you can't do packages.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've considered it on and off for a while. I think the value that DIY gets me is being able to pass through arguments that I don't know about. I only rewrite the arguments that have anything to do with paths. The cost of DIY is that I need this everywhere. At this point I think it is net simpler, but I'd be receptive to any argument to the contrary. Like, I wonder if it'd be helpful to force me to list all of the supported arguments in the python file because we plan to drop the perl file one day.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"I've considered it"

Cool. I trust your judgement.

sub = args.next_arg_or_err()
m = re.match('(?P<repo>[^:]+):(?P<branch>[^:]+):(?P<dir>.+)', sub)
if not m:
raise ArgError("Invalid --sub_dir %s" % sub)
sub_dir = realpath(expanduser(m.group('dir')))
if not exists(sub_dir):
raise ArgError("Can't find --sub_dir %s" % sub_dir)
mounted_path = mount_docs_repo_and_dockerify_path(sub_dir, sub_dir)
build_docs_args.append("%s:%s:%s" % (
m.group('repo'), m.group('branch'), mounted_path))
arg = args.next_arg()

if saw_doc and not saw_out:
Expand Down
33 changes: 26 additions & 7 deletions build_docs.pl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ BEGIN

GetOptions(
$Opts, #
'all', 'push', 'target_repo=s', 'reference=s', 'rebuild', 'no_fetch', #
'all', 'push', 'target_repo=s', 'reference=s', 'rebuild', 'keep_hash', 'sub_dir=s@',
'single', 'pdf', 'doc=s', 'out=s', 'toc', 'chunk=i', 'suppress_migration_warnings',
'open', 'skiplinkcheck', 'linkcheckonly', 'staging', 'procs=i', 'user=s', 'lang=s',
'lenient', 'verbose', 'reload_template', 'resource=s@', 'asciidoctor', 'in_standard_docker',
Expand Down Expand Up @@ -464,7 +464,11 @@ sub init_repos {
user => $Opts->{user},
url => $Opts->{target_repo},
reference => $reference_dir,
# intentionally not passing the tracker because we don't want to use it
# We can't keep the hash of the target repo because it is what stores
# the hashes in the first place!
keep_hash => 0,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't keep_hash on the target repo because the target_repo contains the list of hashes to keep!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's worth a review comment, is it worth a code comment?

# Intentionally not passing the tracker because we need to build the
# tracker from information in this repo.
);
delete $child_dirs{ $target_repo->git_dir->absolute };
my $target_repo_checkout = "$temp_dir/target_repo";
Expand Down Expand Up @@ -498,6 +502,7 @@ sub init_repos {
user => $Opts->{user},
url => $url,
reference => $reference_dir,
keep_hash => $Opts->{keep_hash},
);
delete $child_dirs{ $repo->git_dir->absolute };

Expand All @@ -507,7 +512,7 @@ sub init_repos {
else {
$pm->start($name) and next;
eval {
$repo->update_from_remote() unless $Opts->{no_fetch};
$repo->update_from_remote();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I drop this feature because --keep_hash is better in most cases.

1;
} or do {
# If creds are invalid, explicitly reject them to try to clear the cache
Expand All @@ -522,6 +527,16 @@ sub init_repos {
}
$pm->wait_all_children;

# Parse the --sub_dir options and attach the to the repo
my %sub_dirs = ();
foreach (@{ $Opts->{sub_dir} }) {
die "invalid --sub_dir $_"
unless /(?<repo>[^:]+):(?<branch>[^:]+):(?<dir>.+)/;
my $dir = dir($+{dir})->absolute;
die "--sub_dir $dir doesn't exist" unless -e $dir;
ES::Repo->get_repo($+{repo})->add_sub_dir($+{branch}, $dir);
}

for ( keys %child_dirs ) {
my $dir = dir($_);
next unless -d $dir;
Expand Down Expand Up @@ -682,7 +697,8 @@ sub check_args {
die('--user not compatible with --doc') if $Opts->{user};
die('--reference not compatible with --doc') if $Opts->{reference};
die('--rebuild not compatible with --doc') if $Opts->{rebuild};
die('--no_fetch not compatible with --doc') if $Opts->{no_fetch};
die('--keep_hash not compatible with --doc') if $Opts->{keep_hash};
die('--sub_dir not compatible with --doc') if $Opts->{sub_dir};
die('--skiplinkcheck not compatible with --doc') if $Opts->{skiplinkcheck};
die('--linkcheckonly not compatible with --doc') if $Opts->{linkcheckonly};
} else {
Expand All @@ -703,9 +719,10 @@ sub pick_conf {
#===================================
return 'conf.yaml' unless $Opts->{conf};

my $conf = dir($Old_Pwd)->file($Opts->{conf});
my $conf = file($Opts->{conf});
$conf = dir($Old_Pwd)->file($Opts->{conf}) if $conf->is_relative;
return $conf if -e $conf;
die $Opts->{conf} . " doesn't exist";
die "$conf doesn't exist";
}

#===================================
Expand Down Expand Up @@ -793,7 +810,9 @@ sub usage {
--skiplinkcheck Omit the step that checks for broken links
--linkcheckonly Skips the documentation builds. Checks links only.
--rebuild Rebuild all branches of every book regardless of what has changed
--no_fetch Skip fetching updates from source repos
--keep_hash Build docs from the same commit hash as last time
--sub_dir Use a directory as a branch of some repo
(eg --sub_dir elasticsearch:master:~/Code/elasticsearch)
ninaspitfire marked this conversation as resolved.
Show resolved Hide resolved

General Opts:
--staging Use the template from the staging website
Expand Down
183 changes: 160 additions & 23 deletions integtest/Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
SHELL = /bin/bash -eux -o pipefail
MAKEFLAGS += --silent
TMP = /tmp/docs_integtest/$@

# Used by the test for --all
export GIT_AUTHOR_NAME=Test
Expand All @@ -18,7 +19,11 @@ check: \
missing_include_fails_asciidoc missing_include_fails_asciidoctor \
migration_warnings \
readme_expected_files readme_same_files \
small_all_expected_files
simple_all \
relative_conf_file \
keep_hash \
sub_dir \
keep_hash_and_sub_dir

.PHONY: style
style: html_diff
Expand Down Expand Up @@ -122,38 +127,170 @@ migration_warnings: migration_warnings.asciidoc
/tmp/%_asciidoctor:
$(BD) --asciidoctor --doc $*.asciidoc

.PHONY: small_all_expected_files
small_all_expected_files: /tmp/small_all
[ -s $^/redirects.conf ]
[ -s $^/html/branches.yaml ]
grep '<a class="ulink" href="test/current/index.html" target="_top">Test book</a>' $^/html/index.html > /dev/null
grep '<meta http-equiv="refresh" content="0; url=current/index.html">' $^/html/test/index.html > /dev/null
[ -s $^/html/test/current/index.html ]
.PHONY: simple_all
simple_all:
# Test the simplest possible `--all` invocation
rm -rf $(TMP)
$(BUILD_MINIMAL_ALL)
$(MINIMAL_ALL_EXPECTED_FILES)

.PRECIOUS: /tmp/small_all
/tmp/small_all:
# Builds "--all" documentation specified by by the "small_conf.yaml" file.
.PHONY: relative_conf_file
relative_conf_file:
# Make sure that using a relative referece to the --conf file works.
rm -rf $(TMP)
$(SETUP_MINIMAL_ALL)
cd $(TMP) && \
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf conf.yaml
$(MINIMAL_ALL_EXPECTED_FILES)

# First build a repository to use as the source.
rm -rf /tmp/source
git init /tmp/source
cp minimal.asciidoc /tmp/source/
cd /tmp/source && \
.PHONY: keep_hash
keep_hash:
# Test that `--all --keep_hash` doesn't pull new updates
rm -rf $(TMP)
$(BUILD_MINIMAL_ALL)

# REPLACE the minimal documentation in the source repo
cp ../README.asciidoc $(TMP)/source/index.asciidoc
cd $(TMP)/source && \
git add . && \
git commit -m 'README'

# Rebuild the docs with --keep_hash which should ignore the replacement
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml \
--keep_hash | tee $(TMP)/out
$(call GREP,'No changes to push',$(TMP)/out)

# We expact the same files as the minimal because we the changes that we
# make shouldn't be included
$(MINIMAL_ALL_EXPECTED_FILES)

.PHONY: sub_dir
sub_dir:
# Test that `--all --sub_dir` substitutes a directory for a branch of
# a repo.
rm -rf $(TMP)

# We still need to build the source repo because the script wants to fetch
# it just in case we need another branch.
git init $(TMP)/source
cd $(TMP)/source && git commit --allow-empty -m "empty"

# Setup the directory we'd like to substitute
mkdir $(TMP)/to_sub
cp minimal.asciidoc $(TMP)/to_sub/index.asciidoc

git init --bare $(TMP)/dest.git
sed -e 's|--tmp--|$(TMP)|' small_conf.yaml > $(TMP)/conf.yaml
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml \
--sub_dir source:master:$(TMP)/to_sub

$(MINIMAL_ALL_EXPECTED_FILES)

.PHONY: keep_hash_and_sub_dir
keep_hash_and_sub_dir:
# Test that `--all --keep_hash --sub_dir` keeps hashes the same for repos
# not specified by --sub_dir but forces rebuilding all books that include
# --sub_dir.

rm -rf $(TMP)
$(call INIT_REPO_WITH_FILE,$(TMP)/source1,includes_source2.asciidoc,docs/index.asciidoc)
$(call INIT_REPO_WITH_FILE,$(TMP)/source2,included.asciidoc,index.asciidoc)
git init --bare $(TMP)/dest.git

sed 's|--tmp--|$(TMP)|' two_repos_conf.yaml > $(TMP)/conf.yaml
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml

# Move a "bad" file into source2 so we can be sure we're not picking it up
cp ../README.asciidoc $(TMP)/source2/index.asciidoc
cd $(TMP)/source2 && \
git add . && \
git commit -m 'minimal'
git commit -m 'README'

/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml \
--keep_hash | tee /tmp/out
$(call GREP,'No changes to push',/tmp/out)

# Setup the directory we'd like to substitute
mkdir -p $(TMP)/to_sub/docs
cp includes_source2.asciidoc $(TMP)/to_sub/docs/index.asciidoc
echo "extra extra extra" >> $(TMP)/to_sub/docs/index.asciidoc

/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml \
--keep_hash --sub_dir source1:master:$(TMP)/to_sub | tee $(TMP)/out
$(call GREP,'Pushing changes',$(TMP)/out)

git clone $(TMP)/dest.git $(TMP)/dest
$(call GREP,'extra extra extra',$(TMP)/dest/html/test/current/_chapter.html)

define GREP=
# grep for a string in a file, outputting the whole file if there isn't
# a match.
[ -e $(2) ] || { \
echo "can't find $(2)"; \
ls $$(dirname $(2)); \
false; \
}
grep $(1) $(2) > /dev/null || { \
echo "Couldn't find $(1) in $(2):"; \
cat $(2); \
false; \
}
endef

define SETUP_MINIMAL_ALL=
# First build a repository to use as the source.
$(call INIT_REPO_WITH_FILE,$(TMP)/source,minimal.asciidoc,index.asciidoc)

# Initialize a bare repository that the docs build process can use as a
# remote. It is used to pushing to github but it can push to a remote on
# the filesystem just fine.
git init --bare /tmp/small_all.git
git init --bare $(TMP)/dest.git

# Actually build the docs
sed 's|--tmp--|$(TMP)|' small_conf.yaml > $(TMP)/conf.yaml
endef

define BUILD_MINIMAL_ALL=
# Builds `--all` docs using a "minimal" source file
$(SETUP_MINIMAL_ALL)
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo /tmp/small_all.git \
--conf small_conf.yaml
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml
endef

# Check out the files we just built
git clone /tmp/small_all.git /tmp/small_all
define MINIMAL_ALL_EXPECTED_FILES=
# Checks that $(1).git contains the expected result of building the
# "minimal" source file
git clone $(TMP)/dest.git $(TMP)/dest
[ -s $(TMP)/dest/redirects.conf ]
[ -s $(TMP)/dest/html/branches.yaml ]
$(call GREP,'<a class="ulink" href="test/current/index.html" target="_top">Test book</a>',$(TMP)/dest/html/index.html)
$(call GREP,'<meta http-equiv="refresh" content="0; url=current/index.html">',$(TMP)/dest/html/test/index.html)
[ -s $(TMP)/dest/html/test/current/index.html ]
endef

define INIT_REPO_WITH_FILE=
# Initializes the repo at $(1) and commits the file in $(2) with the
# name $(3)
git init $(1)
mkdir -p $$(dirname $(1)/$(3))
cp $(2) $(1)/$(3)
cd $(1) && \
git add . && \
git commit -m 'init'
endef

define GREP=
# grep for a string in a file, outputting the whole file if there isn't
Expand All @@ -168,4 +305,4 @@ define GREP=
cat $(2); \
false; \
}
endef
endef
9 changes: 9 additions & 0 deletions integtest/includes_source2.asciidoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
= Title

== Chapter

I include simple between here

include::../../source2/index.asciidoc[]

and here.
Loading