cros_chrome_sdk (simple chrome): Run gn gen |
|||||
Issue descriptionMultiple developers have requested the ability for Simple Chrome to run 'gn gen' automatically,
,
Sep 19 2017
,
Sep 19 2017
There are a few options we need to consider: 1. Whether running 'gn gen' should be the default. Most developers probably want it enabled, especially if we provide a mechanism for adding developer args, however if we make it the default any existing args.gn files would be destructively overwritten. 2. How to provide additional developer arguments, e.g. 'dcheck_always_on', 'is_debug', 'symbol_level', 'optimize_webui' (formerly 'use_vulcanize'). This will be important for web UI developers and any developers wishing to use a debugger. There are a few options here: a) Use a command line argument. b) Use a Linux environment variable. c) Use a Simple Chrome environment variable or command specified in ~/.chromite/chrome_sdk.bashrc Comments / suggestions / preferences?
,
Sep 19 2017
I think running gn gen should be default. Making the tool opinionated by default makes it simpler for the vast majority of developers, and advanced/custom scenarios have an escape hatch. Having gn gen off by default means that when we tell the user about it, it is already too late. It is easy to forget to add the flag as well. I'd be a little surprised if people heavily customized args.gn, given that the standard update mechanism is to overwrite the file. Power users who have customized args.gn likely know exactly where they deviate from the default, so reconfiguring it should be relatively painless. If we don't turn the flag on by default, we're optimizing for a one-time cost, versus a greater mental burden long-term. A PSA will hopefully make this one-time cost minimal.
,
Sep 19 2017
I think I'm one of the devs who requested this, so I'll chime in. 1. Args are stored per-board, right? Developers who need to set them presumably already have them saved somewhere so they can restore them (e.g. for new boards). 2. I don't have strong opinions about giving people a way to modify $GN_ARGS vs. introducing a new environment variable for extra args, but I'd prefer to not have a command-line flag for this. I suspect that most developers will want to use the same args for every build instead of wanting to be able to specify them every time.
,
Sep 19 2017
Anec-data: The 4 non-chromeos chrome developers I work with never customize args.gn for simplechrome.
,
Sep 20 2017
The consensus for (1) seems to be make "gn gen" the default so I will go ahead and do that (and send a PSA when we update chromite). The preference for (2) seems to be "not a command line arg". FWIW, any Web UI developer will need to modify args.gn, and several such devs are Chrome devs using Simple Chrome (i.e. not "power users"). I am trying to find a fairly simple solution for them. I have a preference for 2a (mostly because it's flexible, doesn't require an extra file, and is simple to implement), but it has been suggested that 2c is an existing mechanism so would be preferable. However the only way that comes to mind to implement 2c is to run "gn gen" after processing ~/.chromite/chrome_sdk.bashrc, which seems like it might be complicated (and is something I don't really have cycles for at the moment). In order to move things along I put up an implementation for just (1): https://chromium-review.googlesource.com/c/chromiumos/chromite/+/675567 I will have to live with running with using --nogn-gen until we decide how best to implement (2).
,
Sep 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/66680126d28265cd47d3a5551d03892ccc100c77 commit 66680126d28265cd47d3a5551d03892ccc100c77 Author: Steven Bennetts <stevenjb@chromium.org> Date: Thu Sep 28 21:29:25 2017 cros_chrome_sdk: Add --gn-gen option This also adds 'optimize_webui' to |args_to_ignore|, replacing 'use_vulcanize'. BUG= chromium:766398 TEST=Run 'cros chrome-sdk' with and without -gn-gen. Change-Id: Ib8f54a28e6a1ae95afe65a50b4d37f271dc1ba2a Reviewed-on: https://chromium-review.googlesource.com/675567 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Achuith Bhandarkar <achuith@chromium.org> Tested-by: Steven Bennetts <stevenjb@chromium.org> [modify] https://crrev.com/66680126d28265cd47d3a5551d03892ccc100c77/cli/cros/cros_chrome_sdk.py
,
Sep 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/2995d34347981e8537814415f48ece2e770eb9d7 commit 2995d34347981e8537814415f48ece2e770eb9d7 Author: Michael Giuffrida <michaelpg@chromium.org> Date: Fri Sep 29 18:18:15 2017 Revert "cros_chrome_sdk: Add --gn-gen option" This reverts commit 66680126d28265cd47d3a5551d03892ccc100c77. Reason for revert: Breaks PFQ's TestSimpleChromeWorkflow stage. crbug.com/770251 Original change's description: > cros_chrome_sdk: Add --gn-gen option > > This also adds 'optimize_webui' to |args_to_ignore|, > replacing 'use_vulcanize'. > > BUG= chromium:766398 > TEST=Run 'cros chrome-sdk' with and without -gn-gen. > > Change-Id: Ib8f54a28e6a1ae95afe65a50b4d37f271dc1ba2a > Reviewed-on: https://chromium-review.googlesource.com/675567 > Reviewed-by: Steven Bennetts <stevenjb@chromium.org> > Reviewed-by: Achuith Bhandarkar <achuith@chromium.org> > Tested-by: Steven Bennetts <stevenjb@chromium.org> Bug: chromium:766398 TBR=stevenjb@chromium.org Change-Id: Idddaf908aaad49da076d4723e5e2bf61e7352ad8 Reviewed-on: https://chromium-review.googlesource.com/693114 Tested-by: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> [modify] https://crrev.com/2995d34347981e8537814415f48ece2e770eb9d7/cli/cros/cros_chrome_sdk.py
,
Nov 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/9ab0a618d8d4c174fac0c0b3f29183e0b4eec11d commit 9ab0a618d8d4c174fac0c0b3f29183e0b4eec11d Author: Steven Bennetts <stevenjb@chromium.org> Date: Mon Nov 13 18:03:20 2017 cros_chrome_sdk: Automatically generate args.gn files With this CL, entering simple chrome will automatically generate the args.gn file instead of just warning the developer. Additioanlly: * Adds --gn-extra-args to provide additional gn args * Adds --nogn-gen to skip auto generation of args.gn (shows a warning instead, identical to current behavior) * Adds 'optimize_webui' to |args_to_ignore|, replacing 'use_vulcanize'. * Correctly supports running from a location other than chrome/src BUG= chromium:766398 TEST=Run 'cros chrome-sdk' with/without --gn-extra-args, --nogn-gen. Also test from outside the chrome/src directory (bots do this) Change-Id: I3d0cb27faf5300c8e86a0234c37075beb45a6bef Reviewed-on: https://chromium-review.googlesource.com/693006 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> Tested-by: Steven Bennetts <stevenjb@chromium.org> [modify] https://crrev.com/9ab0a618d8d4c174fac0c0b3f29183e0b4eec11d/cli/cros/cros_chrome_sdk.py
,
Nov 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/575c52b3e1f9e893254d02f073e61437f99bf3a9 commit 575c52b3e1f9e893254d02f073e61437f99bf3a9 Author: Steven Bennetts <stevenjb@chromium.org> Date: Tue Nov 21 19:08:59 2017 cros_chrome_sdk: Add --gn-gen and make --nogn-gen the default This CL: * Adds --gn-gen so that devs and bots can start using it * Keeps --nogn-gen so that bots can continue to use that * Makes --nogn-gen the default BUG= chromium:766398 TEST=Run 'cros chrome-sdk' with/without --gn-extra-args, --{no}gn-gen. Change-Id: I6456cfc89e44c83b9f4e57a546b444b1b4dc4930 Reviewed-on: https://chromium-review.googlesource.com/778049 Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Tested-by: Steven Bennetts <stevenjb@chromium.org> [modify] https://crrev.com/575c52b3e1f9e893254d02f073e61437f99bf3a9/cli/cros/cros_chrome_sdk.py
,
Nov 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/8984fec55c977835589d92b6ffd0e7111acb452c commit 8984fec55c977835589d92b6ffd0e7111acb452c Author: Steven Bennetts <stevenjb@chromium.org> Date: Mon Nov 27 20:28:55 2017 chrome-sdk: Pass --nogn-gen from builders In order to simplify developer workflow we want to make --gn-gen the default behavior for simple chrome. Since builders explicitly generate args.gn we need to pass --nogn-gen to preserver the current behavior when the default is changed. Bug: 766398 Change-Id: Ide57e04a8b1d34537508c5f33c851770741ed446 Reviewed-on: https://chromium-review.googlesource.com/783888 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Steven Bennetts <stevenjb@chromium.org> [modify] https://crrev.com/8984fec55c977835589d92b6ffd0e7111acb452c/scripts/slave/recipe_modules/chromium/tests/run_mb.expected/cros_board.json [modify] https://crrev.com/8984fec55c977835589d92b6ffd0e7111acb452c/scripts/slave/recipe_modules/chromium/tests/runhooks.expected/chromeos.json [modify] https://crrev.com/8984fec55c977835589d92b6ffd0e7111acb452c/scripts/slave/recipe_modules/filter/tests/analyze.expected/basic.json [modify] https://crrev.com/8984fec55c977835589d92b6ffd0e7111acb452c/scripts/slave/recipe_modules/chromium/api.py [modify] https://crrev.com/8984fec55c977835589d92b6ffd0e7111acb452c/scripts/slave/recipe_modules/chromium/tests/compile.expected/chromeos.json
,
Nov 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/docs/+/5a79a1cdd9c5bec15a980be662e5d8e699c18706 commit 5a79a1cdd9c5bec15a980be662e5d8e699c18706 Author: Steven Bennetts <stevenjb@chromium.org> Date: Tue Nov 28 21:34:30 2017 Update simple_chrome_workflow.md wiht '--gn-gen' For now we want to encourage developers to pass 'gn-gen' to simple chrome (until it becomes the default). We also want to document 'gn-gen' and 'nogn-gen' regardless. BUG= chromium:766398 TEST=../chromium/src/tools/md_browser/md_browser.py -d . Change-Id: Ic9c1544e2fdf17eec706b9be8c62f5f8f11c3712 Reviewed-on: https://chromium-review.googlesource.com/791520 Commit-Ready: Steven Bennetts <stevenjb@chromium.org> Tested-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> [modify] https://crrev.com/5a79a1cdd9c5bec15a980be662e5d8e699c18706/simple_chrome_workflow.md
,
Dec 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/38b7e69faa51c9595cd8a2b429a2b144173f650a commit 38b7e69faa51c9595cd8a2b429a2b144173f650a Author: Steven Bennetts <stevenjb@chromium.org> Date: Mon Dec 11 23:16:26 2017 cros chrome-sdk: Make gn-gen the default Now that the bots pass --nogn-gen to 'cros chrome-sdk' we can make --gn-gen the default for developers. BUG= chromium:766398 TEST=cros_chrome_sdk_unittest; manual testing of 'cros chrome-sdk' with '-gn-gen', '--nogn-gen', and with neither. Change-Id: I16e4f58c9193ca6137585ed844d8ae5af8d5ce68 Reviewed-on: https://chromium-review.googlesource.com/815154 Commit-Ready: Steven Bennetts <stevenjb@chromium.org> Tested-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> [modify] https://crrev.com/38b7e69faa51c9595cd8a2b429a2b144173f650a/cli/cros/cros_chrome_sdk.py
,
Dec 13 2017
,
Jul 30
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by steve...@chromium.org
, Sep 19 2017