Issue 1299 - Generate docs outside the devbuild build process

sitescripts/docs/bin/generate_docs.py

Index: sitescripts/docs/bin/generate_docs.py
===================================================================
new file mode 100644
--- /dev/null
+++ b/sitescripts/docs/bin/generate_docs.py
@@ -0,0 +1,93 @@
+#!/usr/bin/env python
+# coding: utf-8

[Sebastian Noack, 2016/02/05 18:02:32]

Not that a coding declaration is necessary here. Moreover, according to PEP-8
source files (in Python 2) should use the ASCII anyway.

[Felix Dahlke, 2016/02/05 20:47:58]

Wow true, either that part changed or we misunderstood it all along. We have
this coding declaration in 95% of the source files in sitescripts. I'll leave it
out here then, but we should consider changing that across the bank.

+
+# This file is part of Adblock Plus <https://adblockplus.org/>,
+# Copyright (C) 2006-2016 Eyeo GmbH
+#
+# Adblock Plus is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# Adblock Plus is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+import shutil
+import subprocess
+
+from sitescripts.utils import get_config
+
+config = dict(get_config().items("docs"))

[Sebastian Noack, 2016/02/05 18:02:31]

I'd prefer if you use the ConfigParser API consistently rather than turning it
into a dictionary for no obvious reason.

[Felix Dahlke, 2016/02/05 20:47:57]

Done.

+
+def read_projects():
+  projects = {}
+  for key, value in config.iteritems():
+    key_parts = key.split("_", 1)
+    if len(key_parts) < 2:
+      continue
+    project_name, field_name = key_parts
+    if field_name not in ["repository", "target_directory", "command"]:

[Sebastian Noack, 2016/02/05 18:02:32]

Nit: A set seems more appropriate here. In case you didn't know yet there are
also set literals:

  {"repository", "target_directory", "command"}

[Felix Dahlke, 2016/02/05 20:47:58]

No I didn't, nice. Done.

+      continue
+    projects.setdefault(project_name, {})

[Sebastian Noack, 2016/02/05 18:02:31]

There is a reason that setdefault() returns the respective value:

  projects.setdefault(project_name, {})[field_name] = value

[Felix Dahlke, 2016/02/05 20:47:58]

Done.

+    projects[project_name][field_name] = value
+  return projects
+
+def sync_sources(sources_dir, repository_url):
+  remote_id = subprocess.check_output(["hg", "id", "-i", repository_url])

[Sebastian Noack, 2016/02/05 18:02:32]

Nit: Please use long option names in scripts, that people not familiar with the
invoked program have a better idea what it does. (The shorthand syntax mostly
exist for convenience on the shell.)

[Felix Dahlke, 2016/02/05 20:47:58]

Done.

+  id_path = sources_dir.rstrip("/") + ".id"

[Sebastian Noack, 2016/02/05 18:02:31]

Please don't hard-code file separators, use.ospath.sep.

[Felix Dahlke, 2016/02/05 20:47:58]

Done.

+  if os.path.exists(id_path):

[Sebastian Noack, 2016/02/05 18:02:31]

It's better to handle the corresponding IOError, rather than performing checks
in advance. That not only keeps the kernel/IO-roundtrip small, but also avoids
race conditions.

[Felix Dahlke, 2016/02/05 20:47:57]

Fair enough, I've gotten rid of the os.path.exists() I had an idea for how to
get rid of. Ideas welcome for the remaining ones.

+    with open(id_path, "r") as id_file:

[Sebastian Noack, 2016/02/05 18:02:32]

Please always add "b" to the file mode, for consistent behavior on Windows.

[Felix Dahlke, 2016/02/05 20:47:58]

Done.

+      local_id = id_file.read()
+    if local_id == remote_id:
+      return
+  if os.path.exists(sources_dir):
+    shutil.rmtree(sources_dir)
+  subprocess.check_call(["hg", "archive", "-R", repository_url, "-r", "master",
+                         sources_dir])
+
+  # In theory, it is possible that some changesets are pushed after we fetch
+  # the ID above, but before we run `hg archive`, which will lead to an
+  # unnecessary `hg archive` operation the next time this runs.
+  with open(id_path, "w") as id_file:
+    id_file.write(remote_id)
+
+def replace_dir(source_dir, target_dir):
+  if not os.path.exists(target_dir):
+    parent_dir = os.path.dirname(target_dir)
+    if not os.path.exists(parent_dir):
+      os.makedirs(parent_dir)
+    os.rename(source_dir, target_dir)
+  else:
+    old_target_dir = target_dir.rstrip("/") + ".old"
+    if os.path.exists(old_target_dir):
+      shutil.rmtree(old_target_dir)
+    os.rename(target_dir, old_target_dir)
+    os.rename(source_dir, target_dir)
+    shutil.rmtree(old_target_dir)
+
+def run_generation_command(command, sources_dir, output_dir):
+  if os.path.exists(output_dir):
+    shutil.rmtree(output_dir)
+  command = command.format(output_dir=output_dir)
+  subprocess.check_call(command, shell=True, cwd=sources_dir)
+
+def generate_docs(projects):
+  temp_directory = config["temp_directory"]
+  if not os.path.exists(temp_directory):
+    os.makedirs(temp_directory)
+
+  for name, data in projects.iteritems():
+    sources_dir = os.path.join(temp_directory, name)
+    sync_sources(sources_dir, data["repository"])
+    output_dir = sources_dir.rstrip("/") + ".docs"
+    run_generation_command(data["command"], sources_dir, output_dir)
+    replace_dir(output_dir, data["target_directory"])
+
+if __name__ == "__main__":
+  projects = read_projects()
+  generate_docs(projects)