I'm working on a prototype...

biggest concern is enforcing these subdirs as the only ones used.  when it comes to source, it is very easy to forget that some other tools/include files in other dirs should be tracked too.

leveraging the existing CROS_WORKON_SUBDIRS_TO_COPY comes to mind.

Hey Aviv, just assigning you as owner until you find a more suitable owner.

Re #2 agreed in theory, though there are no ebuilds in chromiumos-overlay that use SUBDIRS_TO_COPY (instead, there is some logic that I haven't untangled involving blacklisted subdirs). I wanted to go the separate vars route so that I could get the speedup working on some autotest ebuilds without untangling all of that. I don't anticipate the SUBDIRS_TO_REV feature as being broadly used outside of autotest.

I agree with vapier that we should really enforce that autotest isn't using any of the directories that are skipping revs for.

Can we make this a follow up feature. I don't have the cycles to fix everything that is broken about autotest ebuilds and eclass. If I can land the above change, that at least gives me a hook to prove that this uprev strategy can even save us any time.

As a follow up we can enforce in cros-workon.eclass that SUBDIRS_TO_REV is a superset of SUBDIRS_TO_COPY and have the ebuild fail otherwise.

Ping.

Other investigation -- SUBDIRS_TO_COPY is mutated by autotest.eclass. I don't see a reliable way for portage_util to determine SUBDIRS_TO_COPY just by parsing an ebuild (as it does for other ebuild variables). So, I think that is another argument in favor of my current approach.

As we discussed yesterday, as long as the correctness of SUBDIRS_TO_REV is enforced somehow, I'm all for the change.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/845602e52a05061273e78e3119128ed1ff15f8b6

commit 845602e52a05061273e78e3119128ed1ff15f8b6
Author: Aviv Keshet <akeshet@chromium.org>
Date: Fri Oct 14 06:44:34 2016

cros_mark_as_stable: only rev is rev_subdirs were touched

This adds a portage_util and cros_mark_as_stable feature that is hidden
behind a flag (the flag is not enabled anywhere yet).

The feature causes cros_mark_as_stable to skip uprev of some cros_workon
ebuilds, if it detects that none of the subdirectories that are relevant
to that ebuild have been modified (each ebuild optionally declares such
a set of directories with a CROS_WORKON_SUBDIRS_TO_REV variable).

CQ-DEPEND=CL:399378
BUG= chromium:655884 
TEST=Local tests

Change-Id: Ibcc68e8192abe31ba765915761c1b56ce0d3d84b
Reviewed-on: https://chromium-review.googlesource.com/399539
Commit-Ready: Aviv Keshet <akeshet@chromium.org>
Tested-by: Aviv Keshet <akeshet@chromium.org>
Reviewed-by: Aviv Keshet <akeshet@chromium.org>

[modify] https://crrev.com/845602e52a05061273e78e3119128ed1ff15f8b6/scripts/cros_mark_android_as_stable_unittest.py
[modify] https://crrev.com/845602e52a05061273e78e3119128ed1ff15f8b6/scripts/cros_mark_chrome_as_stable_unittest.py
[modify] https://crrev.com/845602e52a05061273e78e3119128ed1ff15f8b6/scripts/cros_mark_as_stable.py
[modify] https://crrev.com/845602e52a05061273e78e3119128ed1ff15f8b6/lib/portage_util.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/801736c0c2f1aa79b623b7663d09139350cd0e05

commit 801736c0c2f1aa79b623b7663d09139350cd0e05
Author: Aviv Keshet <akeshet@chromium.org>
Date: Fri Oct 14 20:04:33 2016

cros-workon.eclass: add CROS_WORKON_SUBDIRS_TO_REV

BUG= chromium:655884 
TEST=None

Change-Id: Ie7e0264e90cb30be8c2daaee7c2501a2507158d9
Reviewed-on: https://chromium-review.googlesource.com/399378
Commit-Ready: Aviv Keshet <akeshet@chromium.org>
Tested-by: Aviv Keshet <akeshet@chromium.org>
Reviewed-by: Aviv Keshet <akeshet@chromium.org>
Reviewed-by: David James <davidjames@chromium.org>

[modify] https://crrev.com/801736c0c2f1aa79b623b7663d09139350cd0e05/eclass/cros-workon.eclass

Feature is done, though not used anywhere. First candidate likely will be autotest ebuilds. Will require follow up discussion.

Chatted with akeshet@ offline:
We can investigate making the listing of directories to watch somewhat programmatic. Since we have a list of the tests for any particular ebuild, it should be possible to figure out the directories corresponding to that test, rather than manually listing out the tests for each ebuild.

In addition there are certain common autotest directories that each autotest ebuild should uprev on. These can be added to the subdirs watchlist for every ebuild. We can even be conservative in deciding which directories to include in this list.

I'm transferring this bug over to myself for the time being while I investigate, but will keep akeshet@ on CC.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/72a22436ec86cd13d9b41e0aa4c35e594d5d4605

commit 72a22436ec86cd13d9b41e0aa4c35e594d5d4605
Author: Prashant Malani <pmalani@google.com>
Date: Sat Feb 25 18:32:07 2017

Allow for multiple dirs in SUBDIRS_TO_REV

The Ebuild._RunGit() command doesn't tolerate arguments with spaces
very well. This patch constructs the command arguments by appending each
directory in CROS_WORKON_SUBDIRS_TO_REV to the git argument list so
that the command executes correctly.

TEST="cros_mark_as_stable commit --all --boards=veyron_minnie" locally
BUG= chromium:655884 

Change-Id: I8609f112d5d2ccad4cda1e83c0ccdde321626dca
Signed-off-by: Prashant Malani <pmalani@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/437759
Commit-Ready: Prashant Malani <pmalani@google.com>
Tested-by: Prashant Malani <pmalani@google.com>
Reviewed-by: Prashant Malani <pmalani@google.com>

[modify] https://crrev.com/72a22436ec86cd13d9b41e0aa4c35e594d5d4605/lib/portage_util.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/a400afd89c2b269cf8ec8b9dbb6541c44f6cd343

commit a400afd89c2b269cf8ec8b9dbb6541c44f6cd343
Author: Prashant Malani <pmalani@google.com>
Date: Mon Feb 27 20:03:27 2017

Extract subdirs to watch for uprevs from test names

This patch adds a function to generate a list of subdirectories to watch
for changes, while deciding whether to uprev a package's ebuild.

We extract the test names from IUSE_TESTS and use that to find the
directories associated with those tests.

CROS_WORKON_SUBDIRS_TO_REV is still left intact here, but
ShouldRevEBuild() is modified to use the new directory list instead.
This function should probably be made generic, to avoid it being run on
non-test ebuilds.

Currently GetSubdirsToRev() will be called on all ebuilds, but if
IUSE_TESTS isn't defined, this shouldn't cause much of a slowdown, as it
will return an empty list immediately.

Also add a unit test for the test extraction logic of
GetTestsFromSettings().

TEST="cros_mark_as_stable commit --all --boards=veyron_minnie" locally
BUG= chromium:655884 

Change-Id: I7ef061f604e152a74db3f9a9af08f7632b4efb3a
Signed-off-by: Prashant Malani <pmalani@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/439684
Commit-Ready: Prashant Malani <pmalani@google.com>
Tested-by: Prashant Malani <pmalani@google.com>
Reviewed-by: Aviv Keshet <akeshet@chromium.org>

[modify] https://crrev.com/a400afd89c2b269cf8ec8b9dbb6541c44f6cd343/lib/portage_util_unittest.py
[modify] https://crrev.com/a400afd89c2b269cf8ec8b9dbb6541c44f6cd343/lib/portage_util.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/974aeebca0dcfb5e7dfef25c145e9d7a341db5ac

commit 974aeebca0dcfb5e7dfef25c145e9d7a341db5ac
Author: Prashant Malani <pmalani@google.com>
Date: Mon Apr 03 22:41:50 2017

Enable subdir revision checking

Enable the flag which checks whether any modifications have happened in
monitored directories before up-revving packages. This is only relevant
for auto-test packages and should default to a no-op for other packages.

TEST="cros_mark_as_stable commit --all --boards=veyron_minnie" locally
BUG= chromium:655884 

Change-Id: Idd22fde17ef431e67c9d7853db1786cea3ec8a1c
Signed-off-by: Prashant Malani <pmalani@google.com>
Reviewed-on: https://chromium-review.googlesource.com/447239
Reviewed-by: Aviv Keshet <akeshet@chromium.org>

[modify] https://crrev.com/974aeebca0dcfb5e7dfef25c145e9d7a341db5ac/scripts/cros_mark_as_stable.py

Marking as fixed. Will monitor bugs and edge cases in the implementation as and when they arise.