New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 764075 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

CROS_WORKON_SUBDIRS_TO_REV no longer does anything

Project Member Reported by chirantan@chromium.org, Sep 11 2017

Issue description

https://chromium-review.googlesource.com/c/chromiumos/chromite/+/439684 changed the method used to extract subdirs to watch for changes from the CROS_WORKON_SUBDIRS_TO_REV variable to a programmatic method that looks for the IUSE_TESTS variable.

There are still ebuilds that use this variable.  My searching shows that chromeos-config-bsp for reef, kahlee, and coral boards as well as coreboot-sdk still set CROS_WORKON_SUBDIRS_TO_REV.

What's the long term plan for this functionality?  If it's only supposed to be used for autotest ebuilds then we should remove that variable from the cros-workon eclass so that people don't mistakenly depend on it.  If it's supposed to be used for non-autotest ebuild then we should add the functionality back in.

 
Components: Infra>Client>ChromeOS
I should also add that this functionality would be quite useful for platform2 as well so that we don't end up rebuilding powerd because someone made a change to crosh.
Cc: martinroth@chromium.org
Another use case would be to depend on the files/ directory in the ebuild so that people wouldn't have to remember to make comment-only changes to the -9999.ebuild.

Comment 4 by sjg@google.com, Sep 12 2017

Cc: sjg@chromium.org
Labels: -Pri-2 Pri-1
Eek, we do rely on that feature in unified builds. I did not know it was going away. We need to be able to adjust files in an ebuild's filesdir and have the ebuild rev automatically. Otherwise it is too painful for people to make changes, since they will often conflict. Changes will be made by SIEs and many partners.

Comment 5 by sjg@chromium.org, Sep 13 2017

Cc: jclinton@chromium.org
See my proposed CL here:

https://chromium-review.googlesource.com/c/chromiumos/chromite/+/664484

Comment 6 by vapier@chromium.org, Sep 14 2017

Cc: pbe...@chromium.org yueherngl@chromium.org
 Issue 764544  has been merged into this issue.
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 18 2017

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

commit 722c867ef94b5c4ceb1de3e6279ce6302dbfae47
Author: Simon Glass <sjg@chromium.org>
Date: Mon Sep 18 23:48:45 2017

Reinstate the CROS_WORKON_SUBDIRS_TO_REV functionality

This was removed in an earlier commit [1] but is in fact used by several
ebuilds. Reinstate the variable whiile leaving the test functionality
intact.

[1] https://chromium-review.googlesource.com/c/chromiumos/chromite/+/439684

BUG= chromium:764075 
TEST=manual:
$ cd ~/cosarm/
make a change in src/private-overlays/overlay-coral-private/\
   chromeos-base/chromeos-config-bsp/files/model.dtsi
drop old stabilizing_branch in case you ran this test before
$ git branch -D stabilizing_branch
$ cd src/scripts
$ cros_mark_as_stable commit --all --boards=coral --dryrun \
    -o ../private-overlays/overlay-coral-private
See that it prints:
14:57:50: INFO:  Rev: Determined that one+ of the ebuild chromeos-config-bsp rev_subdirs was touched []
14:57:50: INFO: Committing changes with commit message: Marking 9999 ebuild for chromeos-base/chromeos-config-bsp as stable.

and writes a commit to the overlay repo.

Also:
$ scripts/cros_mark_as_stable_unittest
(which seems to have no tests relevant to this?)

Signed-off-by: Simon Glass <sjg@chromium.org>

Change-Id: I80a46361228c2e36a9117dcfb3666f58f9497299
Reviewed-on: https://chromium-review.googlesource.com/664484
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

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

Comment 8 by sjg@chromium.org, Sep 19 2017

Owner: sjg@chromium.org
Status: Started (was: Available)
RE #4 This feature is for fine-grained control of which subdirectories will cause an ebuild to rev. In the feature's absence, any change to the relevant repository should cause a rev.

pmalani@ has been doing active development on this feature and its use by autotest ebuilds. I was not aware that it was being used anywhere else yet.
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 21 2017

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

commit 3c1329ab5a0c5c03d8db67006d4051eb16709fae
Author: Simon Glass <sjg@chromium.org>
Date: Thu Sep 21 01:43:00 2017

Expand uprev testing with CROS_WORKON_SUBDIRS_TO_REV

As mentioned in CL:671128 there are some cases not covered by the current
tests. Add a few more and expand the explanation a little.

BUG= chromium:764075 
TEST=./lib/portage_util_unittest

Change-Id: I8031ec96714f01d69251ffa03d590182fce94d36
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/673123
Reviewed-by: Jason Clinton <jclinton@chromium.org>

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

Comment 11 by sjg@chromium.org, Sep 27 2017

Status: Fixed (was: Started)
It looks like https://chrome-internal-review.googlesource.com/459792 went through OK.

Looking at the master paladin log:

https://luci-milo.appspot.com/buildbot/chromeos/master-paladin/16335

03:53:31: INFO:  Rev: Determined that one+ of the ebuild chromeos-config-bsp rev_subdirs was touched []
03:53:31: INFO: Committing changes with commit message: Marking 9999 ebuild for chromeos-base/chromeos-config-bsp as stable.

From the coral paladin:

https://luci-milo.appspot.com/buildbot/chromeos/coral-paladin/1198

04:40:13: INFO: CL:*459792 affects packages chromeos-base/chromeos-firmware-coral, chromeos-base/chromeos-config-bsp

Later on the firmware version shows as:

BIOS version: Google_Coral.9959.0.0

So I think this is fixed.

Sign in to add a comment