Skip to content

Skip post-book actions if we can #673

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

Merged
merged 8 commits into from
Mar 18, 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
1 change: 1 addition & 0 deletions build_docs.pl
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ sub build_all {
say "Skipping documentation builds."
}
else {
say "Building docs";
build_entries( $build_dir, $temp_dir, $toc, @$contents );

say "Writing main TOC";
Expand Down
134 changes: 110 additions & 24 deletions integtest/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ check: \
keep_hash \
sub_dir \
keep_hash_and_sub_dir \
multi_branch \
open_all

.PHONY: style
Expand Down Expand Up @@ -170,6 +171,7 @@ keep_hash:
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml \
--keep_hash | tee $(TMP)/out
$(call GREP_V,'Test book',$(TMP)/out)
$(call GREP,'No changes to push',$(TMP)/out)

# We expact the same files as the minimal because we the changes that we
Expand All @@ -196,8 +198,10 @@ sub_dir:
/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

--sub_dir source:master:$(TMP)/to_sub | tee $(TMP)/out
$(call GREP,'Test book: Building master...',$(TMP)/out)
$(call GREP,'Test book: Finished master',$(TMP)/out)
$(call GREP,'Test book: Copying master to current',$(TMP)/out)
$(call MINIMAL_ALL_EXPECTED_FILES,'local changes')

.PHONY: keep_hash_and_sub_dir
Expand All @@ -214,7 +218,11 @@ keep_hash_and_sub_dir:
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
--conf $(TMP)/conf.yaml | tee $(TMP)/out
$(call GREP,'Test book: Building master...',$(TMP)/out)
$(call GREP,'Test book: Finished master',$(TMP)/out)
$(call GREP,'Test book: Copying master to current',$(TMP)/out)
$(call GREP,'Pushing changes',$(TMP)/out)

# Move a "bad" file into source2 so we can be sure we're not picking it up
cp ../README.asciidoc $(TMP)/source2/index.asciidoc
Expand All @@ -225,8 +233,9 @@ keep_hash_and_sub_dir:
/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)
--keep_hash | tee $(TMP)/out
$(call GREP_V,'Test book',$(TMP)/out)
$(call GREP,'No changes to push',$(TMP)/out)

# Setup the directory we'd like to substitute
mkdir -p $(TMP)/to_sub/docs
Expand All @@ -237,11 +246,84 @@ keep_hash_and_sub_dir:
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml \
--keep_hash --sub_dir source1:master:$(TMP)/to_sub | tee $(TMP)/out
$(call GREP,'Test book: Building master...',$(TMP)/out)
$(call GREP,'Test book: Finished master',$(TMP)/out)
$(call GREP,'Test book: Copying master to current',$(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)

.PHONY: multi_branch
multi_branch:
# Tests for how we rebuild books with multiple branches

# When the book hasn't been built before we build all branches
rm -rf $(TMP)
$(call INIT_REPO_WITH_FILE,$(TMP)/source,minimal.asciidoc,docs/index.asciidoc)
cd $(TMP)/source && git checkout -b prev
git init --bare $(TMP)/dest.git

sed 's|--tmp--|$(TMP)|' multi_branch.yaml > $(TMP)/conf.yaml
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml | tee $(TMP)/out
$(call GREP,'Test book: Building master...',$(TMP)/out)
$(call GREP,'Test book: Finished master',$(TMP)/out)
$(call GREP,'Test book: Building prev...',$(TMP)/out)
$(call GREP,'Test book: Finished prev',$(TMP)/out)
$(call GREP,'Test book: Copying master to current',$(TMP)/out)
$(call GREP,'Test book: Writing versions TOC',$(TMP)/out)
$(call GREP,'Pushing changes',$(TMP)/out)

git clone $(TMP)/dest.git $(TMP)/dest
# The main index links to the current version
$(call GREP,'<a class="ulink" href="test/current/index.html" target="_top">Test book \[master\]</a>',$(TMP)/dest/html/index.html)
$(call GREP,'<a class="ulink" href="test/index.html" target="_top">other versions</a>',$(TMP)/dest/html/index.html)
# And the book's index links to all versions
$(call GREP,'<a class="ulink" href="current/index.html" target="_top">Test book: master (current)</a>',$(TMP)/dest/html/test/index.html)
$(call GREP,'<a class="ulink" href="prev/index.html" target="_top">Test book: prev</a>',$(TMP)/dest/html/test/index.html)
# And all versions have been built
[ -s $(TMP)/dest/html/test/current/index.html ]
[ -s $(TMP)/dest/html/test/master/index.html ]
[ -s $(TMP)/dest/html/test/prev/index.html ]

# When one of the non-current branches has changed we rebuild it but do not
# copy the current branch
cd $(TMP)/source && \
git checkout prev && \
echo "changed" >> docs/index.asciidoc && \
git add . && \
git commit -m "change"
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml | tee $(TMP)/out
$(call GREP_V,'Test book: Building master...',$(TMP)/out)
$(call GREP_V,'Test book: Finished master',$(TMP)/out)
$(call GREP,'Test book: Building prev...',$(TMP)/out)
$(call GREP,'Test book: Finished prev',$(TMP)/out)
$(call GREP_V,'Test book: Copying master to current',$(TMP)/out)
$(call GREP,'Test book: Writing versions TOC',$(TMP)/out)
$(call GREP,'Pushing changes',$(TMP)/out)

# When the current branch has changed we rebuild it and copy it to the
# current branch
cd $(TMP)/source && \
git checkout master && \
echo "changed" >> docs/index.asciidoc && \
git add . && \
git commit -m "change"
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml | tee $(TMP)/out
$(call GREP,'Test book: Building master...',$(TMP)/out)
$(call GREP,'Test book: Finished master',$(TMP)/out)
$(call GREP_V,'Test book: Building prev...',$(TMP)/out)
$(call GREP_V,'Test book: Finished prev',$(TMP)/out)
$(call GREP,'Test book: Copying master to current',$(TMP)/out)
$(call GREP,'Test book: Writing versions TOC',$(TMP)/out)
$(call GREP,'Pushing changes',$(TMP)/out)

.PHONY: open_all
open_all:
# Test that `--all --open` starts nginx in a usable way.
Expand All @@ -259,21 +341,6 @@ open_all:
$(call GREP,'Location: http://localhost:8000/guide/en/elasticsearch/reference/current/setup.html',$(TMP)/guide/rdir)
kill -QUIT $$(cat /run/nginx/nginx.pid)

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)
Expand All @@ -292,7 +359,11 @@ define BUILD_MINIMAL_ALL=
$(SETUP_MINIMAL_ALL)
/docs_build/build_docs.pl --in_standard_docker --all --push \
--target_repo $(TMP)/dest.git \
--conf $(TMP)/conf.yaml
--conf $(TMP)/conf.yaml | tee $(TMP)/out
$(call GREP,'Test book: Building master...',$(TMP)/out)
$(call GREP,'Test book: Finished master',$(TMP)/out)
$(call GREP,'Test book: Copying master to current',$(TMP)/out)
$(call GREP,'Test book: Writing redirect to current branch...',$(TMP)/out)
endef

define MINIMAL_ALL_EXPECTED_FILES=
Expand Down Expand Up @@ -323,13 +394,28 @@ 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)"; \
echo "can't find \'$(2)\'"; \
ls $$(dirname $(2)); \
false; \
}
grep $(1) $(2) > /dev/null || { \
echo "Couldn't" find $(1) in $(2):; \
grep -q $(1) $(2) || { \
echo "Couldn't" find \'$(1)\' in \'$(2)\':; \
cat $(2); \
false; \
}
endef

define GREP_V=
# grep for a string in a file, outputting the whole file if there *is*
# a match.
[ -e $(2) ] || { \
Copy link

Choose a reason for hiding this comment

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

Is the false return the most important factor here, or the text output?

To put it another way, is this function assert_not_in_file or print_file_if_unmatched?

Copy link
Member Author

Choose a reason for hiding this comment

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

assert_not_in_file. But I need to print the file so I can figure out what is going on.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think assert_not_in_file is a better name for this? I'm so used to thinking about grep -v but the output on failure is different.

Copy link

Choose a reason for hiding this comment

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

I don't think it's a big deal, but after reading the function name and the comment, I still wasn't sure which part of the function was the core and which was the side-effect. It's not a big public API, so just tweaking the comment a bit would probably be fine, too.

I don't love the name GREP_V, because it reveals the implementation, but again, this a Makefile, not some hallowed, live-with-it-forever public API.

My question was mostly to satisfy my curiosity, rather than to demand changes.

echo "can't find \'$(2)\'"; \
ls $$(dirname $(2)); \
false; \
}
grep -qv $(1) $(2) || { \
echo Found \'$(1)\' in \'$(2)\':; \
cat $(2); \
false; \
}
endef
54 changes: 54 additions & 0 deletions integtest/multi_branch.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# This is a small config file for build_docs.pl used for testing

# This block is required even when building a single doc
template:
path: .template/
branch:
default:
base_url: 'https://www.elastic.co/'
template_url: 'https://www.elastic.co/guide_template'
staging:
base_url: 'https://stag-www.elastic.co/'
template_url: 'https://stag-www.elastic.co/guide_template'
defaults:
POSTHEAD: |
<link rel="stylesheet" type="text/css" href="styles.css" />
FINAL: |
<script type="text/javascript" src="docs.js"></script>
<script type='text/javascript' src='https://cdn.rawgit.com/google/code-prettify/master/loader/run_prettify.js?lang=yaml'></script>

paths:
# These two configure where to put things in the --target_repo. The
# paths are resolved relative to the root of the --target_repo.
build: html/
branch_tracker: html/branches.yaml
# This configures which directory to use for cloning repos. Because
# these are big we tend to want them to be in a non-temporary but
# .gitignored spot. The path is resolved relative to the root of the
# docs repo.
repos: --tmp--/repos/

# This configures all of the repositories used to build the docs
repos:
# Normally we use the `https://` prefix to clone from github but this file
# is for testing so use a string that we can find with sed and replace with
# a file.
source: --tmp--/source

# The title to use for the table of contents
contents_title: Elastic Stack and Product Documentation

# The actual books to build
contents:
-
title: Test book
prefix: test
current: master
branches: [ master, prev ]
index: docs/index.asciidoc
tags: test tag
subject: Test
sources:
-
repo: source
path: docs/index.asciidoc
Loading