Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(57)

Unified Diff: .gn

Issue 1905433002: Add documentation for exec_script and gypi_to_gn (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: . Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | build/gypi_to_gn.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: .gn
diff --git a/.gn b/.gn
index 2f343e07f5acde756ebbc298969c36f3c54688a1..16c1c38df0670cdb55374ef01db7a13471ca1491 100644
--- a/.gn
+++ b/.gn
@@ -139,7 +139,65 @@ check_targets = [
# These are the list of GN files that run exec_script. This whitelist exists
# to force additional review for new uses of exec_script, which is strongly
-# discouraged except for gypi_to_gn calls.
+# discouraged.
+#
+# GYPI_TO_GN
+#
+# Most of these entries are for gypi_to_gn calls. We should not be adding new
+# calls to this script in the build (see //build/gypi_to_gn.py for detailed
+# advice). The only time you should be editing this list for gypi_to_gn
+# purposes is when moving an existing call to a different place.
+#
+# PLEASE READ
+#
+# You should almost never need to add new exec_script calls. exec_script is
+# slow, especially on Windows, and can cause confusing effects. Although
+# individually each call isn't slow or necessarily very confusing, at the scale
+# of our repo things get out of hand quickly. By strongly pushing back on all
+# additions, we keep the build fast and clean. If you think you need to add a
+# new call, please consider:
+#
+# - Do not use a script to check for the existance of a file or directory to
+# enable a different mode. Instead, use GN build args to enable or disable
+# functionality and set options. An example is checking for a file in the
+# src-internal repo to see if the corresponding src-internal feature should
+# be enabled. There are several things that can go wrong with this:
+#
+# - It's mysterious what causes some things to happen. Although in many cases
+# such behavior can be conveniently automatic, GN optimizes for explicit
+# and obvious behavior so people can more easily diagnose problems.
+#
+# - The user can't enable a mode for one build and not another. With GN build
+# args, the user can choose the exact configuration of multiple builds
+# using one checkout. But implicitly basing flags on the state of the
+# checkout, this functionality is broken.
+#
+# - It's easy to get stale files. If for example the user edits the gclient
+# to stop checking out src-internal (or any other optional thing), it's
+# easy to end up with stale files still mysteriously triggering build
+# conditions that are no longer appropriate (yes, this happens in real
+# life).
+#
+# - Do not use a script to iterate files in a directory (glob):
+#
+# - This has the same "stale file" problem as the above discussion. Various
+# operations can leave untracked files in the source tree which can cause
+# surprising effects.
+#
+# - It becomes impossible to use "git grep" to find where a certain file is
+# referenced. This operation is very common and people really do get
+# confused when things aren't listed.
+#
+# - It's easy to screw up. One common case is a build-time script that packs
+# up a directory. The author notices that the script isn't re-run when the
+# directory is updated, so adds a glob so all the files are listed as
+# inputs. This seems to work great... until a file is deleted. When a
+# file is deleted, all the inputs the glob lists will still be up-to-date
+# and no command-lines will have been changed. The action will not be
+# re-run and the build will be broken. It is possible to get this correct
+# using glob, and it's possible to mess it up without glob, but globs make
+# this situation much easier to create. if the build always lists the
+# files and passes them to a script, it will always be correct.
exec_script_whitelist = [
"//android_webview/BUILD.gn",
"//ash/BUILD.gn",
« no previous file with comments | « no previous file | build/gypi_to_gn.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698