Skip to content

Commit 0a732a9

Browse files
authored
Redo Fancy --sub_dir and --keep_hash (#1273)
Revert "Revert "Revert "Revert "Fancy --sub_dir and --keep_hash (#1262)"""" Adds support building from references We try to do a ton of `git` operations inside the docker container while building docs. Without this change if one of those operations points to a git repo that was created with `--reference` but wasn't `--dissociate`d then we'd fail because with didn't mount the reference. This mounts the reference into the docker container *exactly* where they exist on the host system. Doing anything less makes git very unhappy.
1 parent 3dbdd5e commit 0a732a9

10 files changed

+389
-41
lines changed

README.asciidoc

+11-3
Original file line numberDiff line numberDiff line change
@@ -231,13 +231,21 @@ To check links before you merge your changes:
231231
into the master branch of the `elasticsearch` repo, run:
232232
+
233233
----------------------------
234-
./docs/build_docs --all --target_repo [email protected]:elastic/built-docs.git --open --sub_dir elasticsearch:master:./elasticsearch
234+
./docs/build_docs --all --target_repo [email protected]:elastic/built-docs.git \
235+
--open --keep_hash --sub_dir elasticsearch:master:./elasticsearch
235236
----------------------------
236237

237-
To run a full build to mimic the website build, omit the `--sub_dir` option:
238+
NOTE: If there are no outstanding changes in the `../elasticsearch` directory
239+
then this will build against the result of merging the last successful
240+
docs build and the contents of `../elasticsearch`. If there *are*
241+
outstanding changes then it'll just build against the contents of
242+
`../elasticsearch`.
243+
244+
To run a full build to mimic the website build, omit the `--sub_dir` and
245+
`--keep_hash` options:
238246

239247
----------------------------
240-
build_docs --all --target_repo [email protected]:elastic/built-docs.git --open
248+
./build_docs --all --target_repo [email protected]:elastic/built-docs.git --open
241249
----------------------------
242250

243251
The first time you run a full build is slow as it needs to:

build_docs

+31-3
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ def build_docker_image():
5353
docker_logger.info('Building the docker image that will build the docs. ' +
5454
'Expect this to take somewhere between a hundred ' +
5555
'milliseconds and five minutes.')
56+
# TODO we can probably drop opening the dockerfile
5657
with open(join(DIR, 'Dockerfile')) as dockerfile:
5758
# We attempt to spool up the output from docker build so we can hide it
5859
# if the command is successful *and* runs quickly. If it takes longer
@@ -132,16 +133,17 @@ def run_build_docs(args):
132133

133134
mounted_doc_repo_roots = set()
134135
mounted_doc_repo_names = set()
136+
mounted_alternates = set()
135137

136-
def mount_docs_repo_and_dockerify_path(repo_search_path, path):
138+
def mount_docs_repo_and_dockerify_path(path_in_repo, path):
137139
"""Adds a mount for the root of the repository into the docker
138140
container and rewrites the path so that it is inside that mount.
139141
140142
If the repo happens to already be mounted we won't add two mounts.
141143
"""
142144
repo_root = subprocess.check_output(
143145
['git', 'rev-parse', '--show-toplevel'],
144-
cwd=repo_search_path)
146+
cwd=path_in_repo)
145147
repo_root = repo_root.decode('utf-8').strip()
146148
repo_name = basename(repo_root)
147149
if 'x-pack-' in repo_name:
@@ -168,6 +170,7 @@ def run_build_docs(args):
168170
'-v',
169171
'%s:%s:ro,cached' % (repo_root, repo_mount)
170172
])
173+
mount_alternates(path_in_repo)
171174
if not path.startswith(repo_root):
172175
cmd = 'git rev-parse --show-toplevel'
173176
err = ("provided path doesn't contain `%s`:\n"
@@ -178,6 +181,31 @@ def run_build_docs(args):
178181
raise ArgError(err)
179182
return repo_mount + path[len(repo_root):]
180183

184+
def mount_alternates(path_in_repo):
185+
git_dir = subprocess.check_output(
186+
['git', 'rev-parse', '--git-dir'],
187+
cwd=path_in_repo)
188+
git_dir = git_dir.decode('utf-8').strip()
189+
alternates_file = git_dir + '/objects/info/alternates'
190+
if not exists(alternates_file):
191+
return
192+
193+
with open(alternates_file, 'r') as f:
194+
for line in f.readlines():
195+
# Sadly, the alternates have to be mounted in the docker image
196+
# in *exactly* the same spot where they are in the host
197+
# filesystem or else git will not be able to find them!
198+
alternates_root = line.strip()
199+
if alternates_root in mounted_alternates:
200+
continue
201+
mounted_alternates.add(alternates_root)
202+
docker_args.extend([
203+
'-v',
204+
'%s:%s:ro,cached' % (alternates_root, alternates_root)
205+
])
206+
logger.info("mounted " + alternates_root)
207+
mount_alternates(alternates_root)
208+
181209
open_browser = False
182210
args = Args(args)
183211
saw_doc = False
@@ -313,7 +341,7 @@ def run_build_docs(args):
313341
cmd.extend(docker_args)
314342
cmd.extend([DOCKER_TAG, '/docs_build/build_docs.pl'])
315343
cmd.extend(build_docs_args)
316-
# Use a PIPE for stdin so if our process dies then the docs build sees
344+
# Use a PIPE for stdin so if our process dies then the docs build sees
317345
# stdin close which it will use as a signal to die.
318346
docker_run = common_popen(cmd, subprocess.PIPE)
319347

build_docs.pl

+2-2
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ sub init_repos {
607607
tracker => $tracker,
608608
url => $url,
609609
reference => $reference_dir,
610-
keep_hash => $Opts->{keep_hash},
610+
keep_hash => $Opts->{keep_hash} || 0,
611611
);
612612
delete $child_dirs{ $repo->git_dir->absolute };
613613

@@ -644,7 +644,7 @@ sub init_repos {
644644
ES::DocsRepo->new(
645645
tracker => $tracker,
646646
dir => $conf->{docs} || '/docs_build',
647-
keep_hash => $Opts->{keep_hash}
647+
keep_hash => $Opts->{keep_hash} || 0
648648
);
649649

650650
return $tracker;

integtest/spec/all_books_change_detection_spec.rb

+5-2
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,7 @@ def chapter(index)
401401
before_second_build: lambda do |src, config|
402402
repo = src.repo 'repo'
403403
repo.write 'dummy', 'dummy'
404+
repo.commit 'dummy'
404405

405406
config.extra do |conversion|
406407
conversion.keep_hash.sub_dir(repo, 'master')
@@ -619,7 +620,9 @@ def chapter(index)
619620
book = src.book 'Test'
620621
book.source repo2, 'not_used_actually'
621622
repo = src.repo 'repo'
623+
repo.switch_to_new_branch 'subbed'
622624
repo.write 'index.asciidoc', TWO_CHAPTERS + "\nmore words"
625+
repo.commit 'sub'
623626
config.extra do |conversion|
624627
conversion.keep_hash.sub_dir(repo, 'master')
625628
end
@@ -722,9 +725,9 @@ def self.add_branch(src)
722725
context 'the second build' do
723726
let(:out) { outputs[1] }
724727
include_examples 'commits changes'
725-
it "doesn't print that it is building the original branch" do
728+
it "doesn't print that it is building any branch" do
726729
# The original book hasn't changed so we don't rebuild it
727-
expect(out).not_to include('Test: Building master...')
730+
expect(out).not_to include('Test: Building')
728731
end
729732
end
730733
end
+210
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe 'building all books' do
4+
describe '--sub_dir' do
5+
##
6+
# Setups up a repo that looks like:
7+
# master sub_me
8+
# original master-------->subbed
9+
# |
10+
# new master
11+
# |
12+
# too new master
13+
#
14+
# Optionally runs the build once on the commit "new master". Then it always
15+
# runs the build, substituting sub_me for the master branch. If:
16+
# * --keep_hash isn't specified or
17+
# * the sub_me branch has outstanding changes or
18+
# * there was a merge conflict or
19+
# * the build wasn't run against "new master"
20+
# then --sub_dir will pick up the contents of the directory and the build
21+
# won't have "new maser" because it was forked from "original master".
22+
# Otherwise, --keep_hash and --sub_dir will cause the build to merge
23+
# "new master" and "subbed" and build against *that*.
24+
def self.convert_with_sub(keep_hash: true, commit_sub: true,
25+
build_with_init: true,
26+
cause_merge_conflict: false, premerge: false)
27+
convert_before do |src, dest|
28+
repo = setup_repo src
29+
setup_book src, repo
30+
dest.prepare_convert_all(src.conf).convert if build_with_init
31+
modify_master_after_build repo
32+
setup_sub repo, commit_sub, cause_merge_conflict, premerge
33+
convert src, repo, dest, keep_hash
34+
end
35+
end
36+
37+
def self.setup_repo(src)
38+
repo = src.repo 'repo'
39+
repo.write 'docs/index.adoc', index
40+
repo.write 'docs/from_master.adoc', 'original master'
41+
repo.write 'docs/from_subbed.adoc', 'unsubbed'
42+
repo.commit 'original master'
43+
repo.write 'docs/from_master.adoc', 'new master'
44+
repo.write 'docs/conflict', 'from master'
45+
repo.commit 'new master'
46+
repo
47+
end
48+
49+
def self.setup_book(src, repo)
50+
book = src.book 'Test'
51+
book.index = 'docs/index.adoc'
52+
book.source repo, 'docs'
53+
end
54+
55+
def self.modify_master_after_build(repo)
56+
repo.write 'docs/from_master.adoc', 'too new master'
57+
repo.commit 'too new master'
58+
end
59+
60+
def self.setup_sub(repo, commit_sub, cause_merge_conflict, premerge)
61+
repo.switch_to_branch 'HEAD~2'
62+
repo.switch_to_new_branch 'sub_me'
63+
repo.write 'docs/from_subbed.adoc', 'now subbed'
64+
repo.write 'docs/conflict', 'from subbed' if cause_merge_conflict
65+
repo.commit 'subbed' if commit_sub
66+
repo.merge 'master' if premerge
67+
end
68+
69+
def self.convert(src, repo, dest, keep_hash)
70+
builder = dest.prepare_convert_all src.conf
71+
builder.sub_dir repo, 'master'
72+
builder.keep_hash if keep_hash
73+
builder.convert
74+
dest.checkout_conversion
75+
end
76+
77+
def self.index
78+
<<~ASCIIDOC
79+
= Title
80+
81+
[[chapter]]
82+
== Chapter
83+
84+
include::from_master.adoc[]
85+
include::from_subbed.adoc[]
86+
ASCIIDOC
87+
end
88+
89+
let(:logs) { outputs[-1] }
90+
91+
shared_examples 'examples' do |master|
92+
file_context 'raw/test/master/chapter.html' do
93+
it "contains the #{master} master changes" do
94+
expect(contents).to include("<p>#{master} master</p>")
95+
end
96+
it 'contains the subbed changes' do
97+
expect(contents).to include('<p>now subbed</p>')
98+
end
99+
end
100+
end
101+
shared_examples 'contains the original master and subbed changes' do
102+
include_examples 'examples', 'original'
103+
end
104+
shared_examples 'contains the new master and subbed changes' do
105+
include_examples 'examples', 'new'
106+
end
107+
shared_examples 'contains the too new master and subbed changes' do
108+
include_examples 'examples', 'too new'
109+
end
110+
shared_examples 'log merge' do |path|
111+
it "log that it started merging [#{path}]" do
112+
expect(logs).to include(<<~LOGS)
113+
Test: Merging the subbed dir for [repo][master][#{path}] into the last successful build.
114+
LOGS
115+
end
116+
it "log that it merged [#{path}]" do
117+
expect(logs).to include(<<~LOGS)
118+
Test: Merged the subbed dir for [repo][master][#{path}] into the last successful build.
119+
LOGS
120+
end
121+
end
122+
123+
describe 'without --keep_hash' do
124+
convert_with_sub keep_hash: false
125+
it "doesn't log that it won't merge because of uncommitted changes" do
126+
expect(logs).not_to include(<<~LOGS)
127+
Test: Not merging the subbed dir for [repo][master][docs] because it has uncommitted changes.
128+
LOGS
129+
end
130+
include_examples 'contains the original master and subbed changes'
131+
end
132+
describe 'with --keep_hash' do
133+
describe 'when there are uncommitted changes' do
134+
convert_with_sub commit_sub: false
135+
it "logs that it won't merge because of uncommitted changes" do
136+
expect(logs).to include(<<~LOGS)
137+
Test: Not merging the subbed dir for [repo][master][docs] because it has uncommitted changes.
138+
LOGS
139+
end
140+
include_examples 'contains the original master and subbed changes'
141+
end
142+
describe 'when the source is new' do
143+
convert_with_sub build_with_init: false
144+
it "log that it won't merge because the source is new" do
145+
expect(logs).to include(<<~LOGS)
146+
Test: Not merging the subbed dir for [repo][master][docs] because it is new.
147+
LOGS
148+
end
149+
include_examples 'contains the original master and subbed changes'
150+
end
151+
describe 'when the subbed dir can be merged' do
152+
convert_with_sub
153+
include_examples 'log merge', 'docs'
154+
include_examples 'contains the new master and subbed changes'
155+
end
156+
describe 'when the subbed dir has already been merged' do
157+
# This simulates what github will do if you ask it to build the "sha"
158+
# of the merged PR instead of the "head" of the branch.
159+
convert_with_sub premerge: true
160+
include_examples 'log merge', 'docs'
161+
include_examples 'contains the too new master and subbed changes'
162+
end
163+
describe 'when there is a conflict merging the subbed dir' do
164+
convert_with_sub cause_merge_conflict: true
165+
it 'logs that it failed to merge' do
166+
expect(logs).to include(<<~LOGS)
167+
Test: Failed to merge the subbed dir for [repo][master][docs] into the last successful build:
168+
LOGS
169+
end
170+
it 'logs the conflict' do
171+
expect(logs).to include(<<~LOGS)
172+
CONFLICT (add/add): Merge conflict in docs/conflict
173+
LOGS
174+
end
175+
include_examples 'contains the original master and subbed changes'
176+
end
177+
describe 'when there is more than one source using the same repo' do
178+
def self.setup_book(src, repo)
179+
book = src.book 'Test'
180+
book.index = 'docs/index.adoc'
181+
book.source repo, 'docs/index.adoc'
182+
book.source repo, 'docs/from_master.adoc'
183+
book.source repo, 'docs/from_subbed.adoc'
184+
end
185+
convert_with_sub
186+
include_examples 'log merge', 'docs/index.adoc'
187+
include_examples 'log merge', 'docs/from_master.adoc'
188+
include_examples 'log merge', 'docs/from_subbed.adoc'
189+
include_examples 'contains the new master and subbed changes'
190+
end
191+
describe 'when more than one book uses the same source' do
192+
def self.setup_book(src, repo)
193+
%w[Test Test2].each do |name|
194+
book = src.book name
195+
book.index = 'docs/index.adoc'
196+
book.source repo, 'docs'
197+
end
198+
end
199+
convert_with_sub
200+
include_examples 'log merge', 'docs'
201+
it 'logs only one merge' do
202+
# This asserts that we log a single merge. We *should* be using the
203+
# cache instead.
204+
expect(logs).not_to match(/Merged.+Merged/m)
205+
end
206+
include_examples 'contains the new master and subbed changes'
207+
end
208+
end
209+
end
210+
end

integtest/spec/helper/repo.rb

+7
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ def switch_to_new_branch(new_branch)
9393
##
9494
# Checks out a branch.
9595
def switch_to_branch(branch)
96+
# TODO: rename to checkout?
9697
Dir.chdir @root do
9798
sh "git checkout #{branch}"
9899
end
@@ -139,6 +140,12 @@ def push_to(dest)
139140
end
140141
end
141142

143+
def merge(ref)
144+
Dir.chdir @root do
145+
sh "git merge #{ref}"
146+
end
147+
end
148+
142149
def copy_shared_conf
143150
FileUtils.mkdir_p @root
144151
sh "cp -r /docs_build/shared #{@root}"

0 commit comments

Comments
 (0)