Issue 1299 - Generate docs outside the devbuild build process

sitescripts/docs/bin/generate_docs.py

 #!/usr/bin/env python
 
 # 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
 
-def read_projects():
+def read_projects(config):
   projects = {}
-  for key, value in get_config().items("docs"):
+  for key, value in config.items("docs"):
     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"}:
       continue
     projects.setdefault(project_name, {})[field_name] = value
   return projects
 
 def sync_sources(sources_dir, repository_url):
-  remote_id = subprocess.check_output(["hg", "id", "--id", repository_url])
-  id_path = sources_dir.rstrip(os.path.sep) + ".id"
-  try:
-    with open(id_path, "rb") as id_file:
-      local_id = id_file.read()
-    if local_id == remote_id:
-      return
-  except IOError:
-    pass
-
-  try:
-    shutil.rmtree(sources_dir)
-  except OSError:

[Sebastian Noack, 2016/02/06 09:49:50]

shutil.rmtree() has an ingnore_errors argument.

[Felix Dahlke, 2016/02/06 10:46:40]

Awesome, I was very unhappy with all this boiler plate. Now it's just
os.makedirs that annoys me...

-    pass
-
-  subprocess.check_call(["hg", "archive",
-                         "--repository", repository_url,
-                         "--rev", "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, "wb") as id_file:
-    id_file.write(remote_id)
-
-  # Because of how ensure_dependencies.py works, we have to create fake .hg

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

I discovered that ensure_dependencies.py, being required for `./build.py docs`,
fails if it doesn't find either a `.hg` or `.git` directory. Since we're doing a
shallow clone, there is no such thing. It's easy enough to hack around this, but
I'm not convinced that's the right way to do it. I see two alternatives:

1. We move (back) to using clone and pull here rather than hg archive and the
remote ID check. (Wladimir objected to that in the last patch I wrote for this.)

2. We change ensure_dependencies.py to go for Mercurial by default, but that
won't help us here, it'll take a while before that change has propagated into
adblockplus, adblockpluschrome, adblockpluscore and libadblockplus.

I'm somewhat leaning towards #2. We could create a goodfirstbug to change this
in ensure_dependencies.py and refer to this piece of code from that review, so
it gets removed later.

[Sebastian Noack, 2016/02/05 22:46:18]

Moreover, build.py requires buildtools, and if you don't clone the repo and set
the CWD accordingly we rely on the buildtools version from the server anyway. So
how about supporting buildtools natively here as before but alternatively to a
custom command (for libadblockplus). The config therefore could look like:

adblockpluschrome_jsdoc = true
...
libadblockplus_command = make docs

This would also eliminate the boilerplate in adblockpluscore and the /dev/null
hack in the config here.

[Felix Dahlke, 2016/02/06 06:42:55]

Since I invoke the repository's build.py here, that's exactly what happens.
Hm... I'm not sure. It would simplify things, and we had it this way before. On
the other hand, it seems wrong because sitescripts would always use the latest
version of buildtools even if, for some reason, some project needs an older
version.

Wladimir, thoughts?

[Sebastian Noack, 2016/02/06 09:49:50]

Yeah, I missed something. You are right. That should be fine.
Note that this currently also the case for the nightlies generation. But I
wouldn't insist. Changing ensure_dependencyies.py to use mercurial if running
outside of an repo sounds good too.

[Felix Dahlke, 2016/02/06 10:46:40]

Done.

-  # directories in each source directory.
-  os.makedirs(os.path.join(sources_dir, ".hg"))
+  if os.path.exists(sources_dir):
+    subprocess.check_call(["hg", "pull", "--quiet",
+                           "--rev", "master",
+                           "--repository", sources_dir])
+    subprocess.check_call(["hg", "update", "--quiet",
+                           "--rev", "master"])
+  else:
+    subprocess.check_call(["hg", "clone", "--quiet",
+                           "--updaterev", "master",
+                           repository_url, sources_dir])
 
 def replace_dir(source_dir, target_dir):
   if not os.path.exists(target_dir):
     parent_dir = os.path.dirname(target_dir)
     try:
       os.makedirs(parent_dir)
     except OSError:
       pass
     os.rename(source_dir, target_dir)
   else:
     old_target_dir = target_dir.rstrip(os.path.sep) + ".old"

[Sebastian Noack, 2016/02/06 09:49:50]

Generating backups seems to be unnecessary as the content is auto-generated and
can be reproduced if necessary anyway.

[Felix Dahlke, 2016/02/06 10:46:39]

This isn't really a backup, it's just so that replacing the old with the new
docs is faster, the *.old directory is removed after the rename. That was a
suggestion from Wladimir from the last time I tackled this.

[Sebastian Noack, 2016/02/06 13:39:39]

Acknowledged.

-    try:
-      shutil.rmtree(old_target_dir)
-    except OSError:
-      pass
+    shutil.rmtree(old_target_dir, ignore_errors=True)
     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):
-  try:
-    shutil.rmtree(output_dir)
-  except OSError:
-    pass
+  shutil.rmtree(output_dir, ignore_errors=True)
   command = command.format(output_dir=output_dir)
   subprocess.check_call(command, shell=True, cwd=sources_dir)
 
-def generate_docs(projects):
-  temp_directory = get_config().get("docs", "temp_directory")
+def generate_docs(projects, config):
+  temp_directory = config.get("docs", "temp_directory")
   try:
     os.makedirs(temp_directory)
   except OSError:
     pass
 
   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(os.path.sep) + ".docs"
     run_generation_command(data["command"], sources_dir, output_dir)
     replace_dir(output_dir, data["target_directory"])
 
 if __name__ == "__main__":

[Sebastian Noack, 2016/02/06 09:49:51]

How about calling get_config() here and passing the config to read_projects()
and generate_docs below?

[Felix Dahlke, 2016/02/06 10:46:39]

Done.

-  projects = read_projects()
-  generate_docs(projects)
+  config = get_config()
+  projects = read_projects(config)
+  generate_docs(projects, config)