new_variant: find the upstreamed coreboot CL
After a coreboot patch is merged, it will get upstreamed to the
chromiumos tree as part of a batch. The change-id from the coreboot
gerrit instance will be preserved as original-change-id, so search
for that to find the upstreamed CL in chromiumos gerrit.
Change the messages about doing things outside of new_variant.py
(like generating the fitimage, pushing to coreboot, or waiting for
upstream) from info to error, because these are technically error
conditions that prevent the program from continuing.
BUG=b:150403065
TEST=Check operation when the coreboot CL hasn't been pushed yet,
when it hasn't been upstreamed yet, and when it has been upstreamed.
See testdata/README.md for detailed instructions.
Change-Id: I9f701664d849197cf183fc1fb46f7523095c359c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/dev-util/+/2079691
Tested-by: Paul Fagerburg <pfagerburg@chromium.org>
Tested-by: Martin Roth <martinroth@google.com>
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Paul Fagerburg <pfagerburg@chromium.org>
diff --git a/contrib/variant/new_variant.py b/contrib/variant/new_variant.py
index d88f4b2..fb0be5b 100755
--- a/contrib/variant/new_variant.py
+++ b/contrib/variant/new_variant.py
@@ -253,6 +253,25 @@
we are at.
* yaml_file - internal, just the name of the file where all this data
gets saved.
+ * commit - a map of maps that tracks all of the git commit and gerrit CL
+ data for each of the steps in the process. For example,
+ status.commit['add_priv_yaml'] is a map that has all the information
+ about the 'add_priv_yaml' step. The keys in the maps allow us to
+ determine where the commit is, the change_id, if it has been uploaded
+ to gerrit and where.
+
+ branch_name - the name of the git branch
+ change_id - the change-id assigned by the commit hook. Gerrit
+ uses the change_id to track new patchsets in the CL
+ dir - the directory where the commit has been created
+ gerrit - the name of the gerrit instance to which the CL has
+ been uploaded, one of 'chromium', 'chrome-internal', or
+ 'coreboot'
+ cl_number - the CL number on the gerrit instance
+
+ When the commit is created, branch_name, change_id, and dir are all
+ set. The gerrit and cl_number keys are not set until the CL has been
+ uploaded to a gerrit instance.
These data might come from the status file (because we read it), or
they might be the initial values after we created the file (because
@@ -617,16 +636,16 @@
if not fit_image_files:
return True
- logging.info('The following files need to be generated:')
+ logging.error('The following files need to be generated:')
for filename in fit_image_files:
- logging.info('* %s', filename)
- logging.info('The fitimage sources are ready for gen_fit_image.sh to process.')
- logging.info('gen_fit_image.sh cannot run inside the chroot. Please open a new terminal')
- logging.info('window, change to the directory where gen_fit_image.sh is located, and run')
- logging.info(status.fitimage_cmd, status.variant)
- logging.info('Then re-start this program with --continue.')
- logging.info('If your chroot is based in ~/chromiumos, then the folder you want is')
- logging.info('~/chromiumos/src/%s/asset_generation', status.fitimage_dir)
+ logging.error('* %s', filename)
+ logging.error('The fitimage sources are ready for gen_fit_image.sh to process.')
+ logging.error('gen_fit_image.sh cannot run inside the chroot. Please open a new terminal')
+ logging.error('window, change to the directory where gen_fit_image.sh is located, and run')
+ logging.error(status.fitimage_cmd, status.variant)
+ logging.error('Then re-start this program with --continue.')
+ logging.error('If your chroot is based in ~/chromiumos, then the folder you want is')
+ logging.error('~/chromiumos/src/%s/asset_generation', status.fitimage_dir)
return False
@@ -960,12 +979,12 @@
save_cl_data(status, commit_key, cl)
else:
logging.debug(f'Not found {change_id}, need to upload')
- logging.info('The following commit needs to be pushed to coreboot.org:')
- logging.info(' Branch "%s"', commit['branch_name'])
- logging.info(' in directory "%s"', commit['dir'])
- logging.info(' with change-id "%s"', commit['change_id'])
- logging.info('Please push the branch to review.coreboot.org, '\
- 'and then re-start this program with --continue')
+ logging.error('The following commit needs to be pushed to coreboot.org:')
+ logging.error(' Branch "%s"', commit['branch_name'])
+ logging.error(' in directory "%s"', commit['dir'])
+ logging.error(' with change-id "%s"', commit['change_id'])
+ logging.error('Please push the branch to review.coreboot.org, '
+ 'and then re-start this program with --continue')
# Since this commit needs to be uploaded, do not continue after
# this step returns.
rc = False
@@ -1025,7 +1044,7 @@
# and decode as JSON.
data = json.loads(response[5:])
if '_number' in data:
- return data['_number']
+ return str(data['_number'])
return None
@@ -1145,6 +1164,21 @@
def find_coreboot_upstream(status):
"""Find the coreboot CL after it has been upstreamed to chromiumos
+ When the coreboot variant CL is first uploaded to review.coreboot.org,
+ it is not visible in the chromiumos tree (and also cannot be used as
+ a target for cq-depend). There is a process for upstreaming CLs from
+ coreboot after they have been reviewed, approved, and merged. We can
+ track a specific coreboot CL if we know the change-id that it used on
+ the coreboot gerrit instance, by looking for that change-id as
+ 'original-change-id' in the public chromium gerrit instance.
+
+ The change-id for the coreboot variant will be under the 'cb_variant' key,
+ but this is for the 'coreboot' gerrit instance.
+
+ When we find the upstreamed CL, we will record the gerrit instance and
+ CL number in the yaml file under the 'find' key ("find upstream coreboot")
+ so that we don't need to search coreboot again.
+
Args:
status: variant_status object tracking our board, variant, etc.
@@ -1152,8 +1186,67 @@
True if the build succeeded, False if something failed
"""
logging.info('Running step find_coreboot_upstream')
- del status # unused parameter
- logging.error('TODO (pfagerburg): implement find_coreboot_upstream')
+
+ # If we have already found the upstream coreboot CL, then exit with success
+ if step_names.FIND in status.commits:
+ commit = status.commits[step_names.FIND]
+ if 'gerrit' in commit and 'cl_number' in commit:
+ instance_name = commit['gerrit']
+ cl_number = commit['cl_number']
+ logging.debug(f'Already found ({instance_name}, {cl_number})')
+ return True
+
+ # Make sure we have a CB_VARIANT commit and a change_id for it
+ if step_names.CB_VARIANT not in status.commits:
+ logging.error('Key %s not found in status.commits',
+ step_names.CB_VARIANT)
+ return False
+ if 'change_id' not in status.commits[step_names.CB_VARIANT]:
+ logging.error('Key change_id not found in status.commits[%s]',
+ step_names.CB_VARIANT)
+ return False
+
+ # Find the CL by the Original-Change-Id
+ original_change_id = status.commits[step_names.CB_VARIANT]['change_id']
+ gerrit_query_args = {
+ 'Original-Change-Id': original_change_id
+ }
+ cros = gerrit.GetCrosExternal()
+ upstream = cros.Query(**gerrit_query_args)
+ # If nothing is found, the patch hasn't been upstreamed yet
+ if not upstream:
+ logging.error('Program cannot continue until coreboot CL is upstreamed.')
+ logging.error('(coreboot:%s, change-id %s)',
+ status.commits[step_names.CB_VARIANT]['cl_number'],
+ status.commits[step_names.CB_VARIANT]['change_id'])
+ logging.error('Please wait for the CL to be upstreamed, then run this'
+ ' program again with --continue')
+ return False
+
+ # If more than one CL is found, something is very wrong
+ if len(upstream) != 1:
+ logging.error('More than one CL was found with Original-Change-Id %s',
+ original_change_id)
+ return False
+
+ # At this point, we know there is only one CL and we can get the
+ # repo and CL number by splitting on the colon between them.
+ patchlink = upstream[0].PatchLink()
+ instance_name, cl_number = patchlink.split(':')
+
+ # Can't use get_git_commit_data because we're not pulling this
+ # information from a git commit, but rather from gerrit.
+ # We only need the gerrit instance and the CL number so we can have
+ # other CLs cq-depend on this CL. The other keys are not needed because:
+ # dir - not needed because we're not going to `cd` there to `repo upload`
+ # branch_name - not valid; the CL is already merged
+ # change_id - we use the change_id to find a CL number, and since we
+ # just found the CL number via original-change-id, this is moot.
+ status.commits[step_names.FIND] = {
+ 'gerrit': instance_name,
+ 'cl_number': str(cl_number)
+ }
+
return True
diff --git a/contrib/variant/testdata/README.md b/contrib/variant/testdata/README.md
new file mode 100644
index 0000000..3786060
--- /dev/null
+++ b/contrib/variant/testdata/README.md
@@ -0,0 +1,50 @@
+These instructions show how to test the operation of finding the coreboot
+CL when:
+* the CL hasn't been pushed to review.coreboot.org yet
+* the CL has been pushed, but has not been upstreamed into the chromiumos tree yet
+* the CL has been upstreamed into chromiumos
+
+The test yaml files include the CLs for the creation of the Kindred variant.
+The coreboot CL for Kindred is coreboot:32936, and was upstreamed as
+chromium:1641906. These CLs have long since merged, and so nothing will be
+uploaded to any gerrit instances or merged into ToT.
+
+`need_to_push.yaml` has the change\_id for the coreboot CL modified so that
+the CL cannot be found, which makes it look like the CL has not been pushed.
+The program will ask the user to push it to coreboot.
+
+`need_to_upstream.yaml` also has the change\_id for the coreboot CL modified,
+but the gerrit instance (coreboot) and CL number (32936) are already there,
+so the CL has already been found. However, searching chromium for that
+change\_id as an original-change-id will fail, indicating that the CL has
+not been upstreamed from coreboot yet.
+
+`upstreamed.yaml` has the correct change\_id for the coreboot CL. The program
+will find the CL in coreboot, then find the upstreamed CL in chromium, and
+proceed to the cq\_depend step (which is not yet implemented).
+
+```
+(cr) $ cp testdata/need_to_push.yaml ~/.new_variant.yaml
+(cr) $ ./new_variant.py --continue
+INFO:root:Running step push_coreboot
+INFO:requests.packages.urllib3.connectionpool:Starting new HTTPS connection (1): review.coreboot.org
+ERROR:root:The following commit needs to be pushed to coreboot.org:
+ERROR:root: Branch "kindred"
+ERROR:root: in directory "/mnt/host/source/src/third_party/coreboot"
+ERROR:root: with change-id "Ithischangeidwillnotbefoundbecauseitdoesntexist"
+ERROR:root:Please push the branch to review.coreboot.org, and then re-start this program with --continue
+(cr) $ cp testdata/need_to_upstream.yaml ~/.new_variant.yaml
+(cr) $ ./new_variant.py --continue
+INFO:root:Running step push_coreboot
+INFO:root:Running step upload_CLs
+INFO:root:Running step find_coreboot_upstream
+ERROR:root:Program cannot continue until coreboot CL is upstreamed.
+ERROR:root:(coreboot:32936, change-id Ichangeiddoesntmatterbecausewealreadyknowtheclnumber)
+ERROR:root:Please wait for the CL to be upstreamed, then run this program again with --continue
+(cr) $ cp testdata/upstreamed.yaml ~/.new_variant.yaml
+(cr) $ ./new_variant.py --continue
+INFO:root:Running step push_coreboot
+INFO:root:Running step upload_CLs
+INFO:root:Running step find_coreboot_upstream
+INFO:root:Running step clean_up
+```
diff --git a/contrib/variant/testdata/need_to_push.yaml b/contrib/variant/testdata/need_to_push.yaml
new file mode 100644
index 0000000..b5bd10b
--- /dev/null
+++ b/contrib/variant/testdata/need_to_push.yaml
@@ -0,0 +1,92 @@
+!variant_status
+base: hatch
+board: hatch
+bug: None
+cb_config_dir: null
+commits:
+ add_priv_yaml:
+ branch_name: kindred
+ change_id: I76071e08e495b9c4bbc37e9f173c63e37b1ce7a4
+ cl_number: '1331261'
+ dir: /mnt/host/source/src/private-overlays/overlay-hatch-private
+ gerrit: chrome-internal
+ add_pub_yaml:
+ branch_name: kindred
+ change_id: I721ad038ecc1f5accd84a971fb425a9db1cdd790
+ cl_number: '1629121'
+ dir: /mnt/host/source/src/overlays
+ gerrit: chromium
+ cb_config:
+ branch_name: kindred
+ change_id: I2d0d796c1f3883c5f29c22af1a8bd38ee2a1b139
+ cl_number: '1638243'
+ dir: /mnt/host/source/src/third_party/chromiumos-overlay
+ gerrit: chromium
+ cb_variant:
+ branch_name: kindred
+ change_id: Ithischangeidwillnotbefoundbecauseitdoesntexist
+ dir: /mnt/host/source/src/third_party/coreboot
+ commit_fit:
+ branch_name: kindred
+ change_id: I2816a3bb60920c323e64a9e248930a255f9938ea
+ cl_number: '1364967'
+ dir: /mnt/host/source/src/private-overlays/baseboard-hatch-private
+ gerrit: chrome-internal
+ ec_image:
+ branch_name: kindred
+ change_id: Ie5e976f426537fc0994c43d033a1b595d21095fc
+ cl_number: '1648602'
+ dir: /mnt/host/source/src/platform/ec
+ gerrit: chromium
+coreboot_dir: third_party/coreboot
+coreboot_push_list:
+- cb_variant
+emerge_cmd: emerge-hatch
+emerge_pkgs:
+- coreboot
+- libpayload
+- vboot_reference
+- depthcharge
+- intel-cmlfsp
+- coreboot-private-files-hatch
+- chromeos-ec
+- chromeos-config-bsp-hatch-private
+- chromeos-config
+- chromeos-config-bsp
+- chromeos-config-bsp-hatch
+- coreboot-private-files
+- coreboot-private-files-hatch
+- chromeos-bootimage
+fitimage_cmd: ./gen_fit_image.sh %s <path_to_fit_kit> [-b]
+fitimage_dir: private-overlays/baseboard-hatch-private/sys-boot/coreboot-private-files-hatch
+fitimage_pkg: coreboot-private-files-hatch
+fsp: intel-cmlfsp
+my_loc: /mnt/host/source/src/platform/dev/contrib/variant
+private_yaml_dir: ~/trunk/src/private-overlays/overlay-hatch-private/chromeos-base/chromeos-config-bsp-hatch-private
+repo_upload_list:
+- add_priv_yaml
+- add_pub_yaml
+- cb_config
+- commit_fit
+- ec_image
+step: push
+step_list:
+- push
+- upload
+- find
+- clean_up
+variant: kindred
+workon_pkgs:
+- coreboot
+- libpayload
+- vboot_reference
+- depthcharge
+- intel-cmlfsp
+- coreboot-private-files-hatch
+- chromeos-ec
+- chromeos-config-bsp-hatch-private
+yaml_emerge_pkgs:
+- chromeos-config-bsp
+- chromeos-config
+- chromeos-config-bsp-hatch
+- chromeos-config-bsp-hatch-private
diff --git a/contrib/variant/testdata/need_to_upstream.yaml b/contrib/variant/testdata/need_to_upstream.yaml
new file mode 100644
index 0000000..92155a9
--- /dev/null
+++ b/contrib/variant/testdata/need_to_upstream.yaml
@@ -0,0 +1,94 @@
+!variant_status
+base: hatch
+board: hatch
+bug: None
+cb_config_dir: null
+commits:
+ add_priv_yaml:
+ branch_name: kindred
+ change_id: I76071e08e495b9c4bbc37e9f173c63e37b1ce7a4
+ cl_number: '1331261'
+ dir: /mnt/host/source/src/private-overlays/overlay-hatch-private
+ gerrit: chrome-internal
+ add_pub_yaml:
+ branch_name: kindred
+ change_id: I721ad038ecc1f5accd84a971fb425a9db1cdd790
+ cl_number: '1629121'
+ dir: /mnt/host/source/src/overlays
+ gerrit: chromium
+ cb_config:
+ branch_name: kindred
+ change_id: I2d0d796c1f3883c5f29c22af1a8bd38ee2a1b139
+ cl_number: '1638243'
+ dir: /mnt/host/source/src/third_party/chromiumos-overlay
+ gerrit: chromium
+ cb_variant:
+ branch_name: kindred
+ change_id: Ichangeiddoesntmatterbecausewealreadyknowtheclnumber
+ cl_number: '32936'
+ dir: /mnt/host/source/src/third_party/coreboot
+ gerrit: coreboot
+ commit_fit:
+ branch_name: kindred
+ change_id: I2816a3bb60920c323e64a9e248930a255f9938ea
+ cl_number: '1364967'
+ dir: /mnt/host/source/src/private-overlays/baseboard-hatch-private
+ gerrit: chrome-internal
+ ec_image:
+ branch_name: kindred
+ change_id: Ie5e976f426537fc0994c43d033a1b595d21095fc
+ cl_number: '1648602'
+ dir: /mnt/host/source/src/platform/ec
+ gerrit: chromium
+coreboot_dir: third_party/coreboot
+coreboot_push_list:
+- cb_variant
+emerge_cmd: emerge-hatch
+emerge_pkgs:
+- coreboot
+- libpayload
+- vboot_reference
+- depthcharge
+- intel-cmlfsp
+- coreboot-private-files-hatch
+- chromeos-ec
+- chromeos-config-bsp-hatch-private
+- chromeos-config
+- chromeos-config-bsp
+- chromeos-config-bsp-hatch
+- coreboot-private-files
+- coreboot-private-files-hatch
+- chromeos-bootimage
+fitimage_cmd: ./gen_fit_image.sh %s <path_to_fit_kit> [-b]
+fitimage_dir: private-overlays/baseboard-hatch-private/sys-boot/coreboot-private-files-hatch
+fitimage_pkg: coreboot-private-files-hatch
+fsp: intel-cmlfsp
+my_loc: /mnt/host/source/src/platform/dev/contrib/variant
+private_yaml_dir: ~/trunk/src/private-overlays/overlay-hatch-private/chromeos-base/chromeos-config-bsp-hatch-private
+repo_upload_list:
+- add_priv_yaml
+- add_pub_yaml
+- cb_config
+- commit_fit
+- ec_image
+step: push
+step_list:
+- push
+- upload
+- find
+- clean_up
+variant: kindred
+workon_pkgs:
+- coreboot
+- libpayload
+- vboot_reference
+- depthcharge
+- intel-cmlfsp
+- coreboot-private-files-hatch
+- chromeos-ec
+- chromeos-config-bsp-hatch-private
+yaml_emerge_pkgs:
+- chromeos-config-bsp
+- chromeos-config
+- chromeos-config-bsp-hatch
+- chromeos-config-bsp-hatch-private
diff --git a/contrib/variant/testdata/upstreamed.yaml b/contrib/variant/testdata/upstreamed.yaml
new file mode 100644
index 0000000..b3e499c
--- /dev/null
+++ b/contrib/variant/testdata/upstreamed.yaml
@@ -0,0 +1,95 @@
+!variant_status
+base: hatch
+board: hatch
+bug: None
+cb_config_dir: null
+commits:
+ add_priv_yaml:
+ branch_name: kindred
+ change_id: I76071e08e495b9c4bbc37e9f173c63e37b1ce7a4
+ cl_number: '1331261'
+ dir: /mnt/host/source/src/private-overlays/overlay-hatch-private
+ gerrit: chrome-internal
+ add_pub_yaml:
+ branch_name: kindred
+ change_id: I721ad038ecc1f5accd84a971fb425a9db1cdd790
+ cl_number: '1629121'
+ dir: /mnt/host/source/src/overlays
+ gerrit: chromium
+ cb_config:
+ branch_name: kindred
+ change_id: I2d0d796c1f3883c5f29c22af1a8bd38ee2a1b139
+ cl_number: '1638243'
+ dir: /mnt/host/source/src/third_party/chromiumos-overlay
+ gerrit: chromium
+ cb_variant:
+ branch_name: kindred
+ change_id: I09ad3da0505d599fc3797d7fa24b4dc170dcd18b
+ cl_number: '32936'
+ dir: /mnt/host/source/src/third_party/coreboot
+ gerrit: coreboot
+ commit_fit:
+ branch_name: kindred
+ change_id: I2816a3bb60920c323e64a9e248930a255f9938ea
+ cl_number: '1364967'
+ dir: /mnt/host/source/src/private-overlays/baseboard-hatch-private
+ gerrit: chrome-internal
+ ec_image:
+ branch_name: kindred
+ change_id: Ie5e976f426537fc0994c43d033a1b595d21095fc
+ cl_number: '1648602'
+ dir: /mnt/host/source/src/platform/ec
+ gerrit: chromium
+coreboot_dir: third_party/coreboot
+coreboot_push_list:
+- cb_variant
+emerge_cmd: emerge-hatch
+emerge_pkgs:
+- coreboot
+- libpayload
+- vboot_reference
+- depthcharge
+- intel-cmlfsp
+- coreboot-private-files-hatch
+- chromeos-ec
+- chromeos-config-bsp-hatch-private
+- chromeos-config
+- chromeos-config-bsp
+- chromeos-config-bsp-hatch
+- coreboot-private-files
+- coreboot-private-files-hatch
+- chromeos-bootimage
+fitimage_cmd: ./gen_fit_image.sh %s <path_to_fit_kit> [-b]
+fitimage_dir: private-overlays/baseboard-hatch-private/sys-boot/coreboot-private-files-hatch
+fitimage_pkg: coreboot-private-files-hatch
+fsp: intel-cmlfsp
+my_loc: /mnt/host/source/src/platform/dev/contrib/variant
+private_yaml_dir: ~/trunk/src/private-overlays/overlay-hatch-private/chromeos-base/chromeos-config-bsp-hatch-private
+repo_upload_list:
+- add_priv_yaml
+- add_pub_yaml
+- cb_config
+- commit_fit
+- ec_image
+step: push
+step_list:
+- push
+- upload
+- find
+- cq_depend
+- clean_up
+variant: kindred
+workon_pkgs:
+- coreboot
+- libpayload
+- vboot_reference
+- depthcharge
+- intel-cmlfsp
+- coreboot-private-files-hatch
+- chromeos-ec
+- chromeos-config-bsp-hatch-private
+yaml_emerge_pkgs:
+- chromeos-config-bsp
+- chromeos-config
+- chromeos-config-bsp-hatch
+- chromeos-config-bsp-hatch-private
