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

Issue 807492 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature



Sign in to add a comment

Add -version flag to tast executable

Project Member Reported by derat@chromium.org, Jan 31 2018

Issue description

In his review of https://crrev.com/c/894730/, Ilja suggested logging "tast -version" in the tast_Runner test. I agree, but I'd need to add a -version flag first. We'd probably want to use it to log something analogous to the #define-ed VCSID hash that some platform2 C++ binaries use.

https://stackoverflow.com/questions/11354518/golang-application-auto-build-versioning (and many other pages) suggest using e.g. "-ldflags '-X main.xyz=abc'" to tell the linker to set the value of a string variable.
 
Ability to embed version information into the binary sounds like a good idea.

I'm not sure what level of support would be good to have in the eclass:

a) Very specific: One CROS_GO_VERSION variable, that if set, would cause "-X main.Version=${CROS_GO_VERSION}" to be passed for building all binaries in CROS_GO_BINARIES.

b) Somewhat generic: One CROS_GO_FLAGS variable, that if set, would be passed to all invocations of "go build". This will have the flexibility to specify the "main.xyz" variable name in the ebuilds, or even to use it for build flags other than "-X".

c) Very generic: Extend the "CROS_GO_BINARIES" syntax to specify build flags for each binary, in addition to binary name / install path etc. This will allow arbitrary flags, and the ability to specify different flags for different binary.

My instinct is to add support for (a).
That'll work well enough for now, and we can make it more generic later if needed.

Comment 2 by derat@chromium.org, Feb 2 2018

a) sounds fine to me. I take it that packages could then set CROS_GO_VERSION to contain their package version or repo SHA1 or something similar?
That's what I had in mind. You could simply set CROS_GO_VERSION to ${PV} for packages where upstream has version tags.
Otherwise, just use a shortened form of ${CROS_WORKON_COMMIT} in the ebuild.

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tast/+/db941fe3968b1c4bb089fc6562102b970a32af48

commit db941fe3968b1c4bb089fc6562102b970a32af48
Author: Rahul Chaudhry <rahulchaudhry@chromium.org>
Date: Sat Feb 10 03:03:28 2018

tast: add -version flag.

BUG= chromium:807492 
TEST='sudo emerge chromeos-base/tast-cmd' works.
TEST='tast -version' prints version information.

Change-Id: I5fab7ec3686b148f33debc9bf757f5e7e1844d3c
Reviewed-on: https://chromium-review.googlesource.com/910095
Commit-Ready: Rahul Chaudhry <rahulchaudhry@chromium.org>
Tested-by: Rahul Chaudhry <rahulchaudhry@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/db941fe3968b1c4bb089fc6562102b970a32af48/src/chromiumos/cmd/tast/main.go

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 13 2018

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

commit b88cae8f9b3af0b71bacda3dc187330d5c98fc07
Author: Rahul Chaudhry <rahulchaudhry@chromium.org>
Date: Tue Feb 13 06:42:48 2018

cros-go.eclass: add support for CROS_GO_VERSION variable.

String variable main.Version is set to this value at build time.

BUG= chromium:807492 
TEST='sudo emerge chromeos-base/tast-cmd' works.
TEST='tast -version' prints version information.

Change-Id: I64121b56b6552745968d3c1e199e0c766ef15e63
Reviewed-on: https://chromium-review.googlesource.com/910039
Commit-Ready: Rahul Chaudhry <rahulchaudhry@chromium.org>
Tested-by: Rahul Chaudhry <rahulchaudhry@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/b88cae8f9b3af0b71bacda3dc187330d5c98fc07/eclass/cros-go.eclass
[modify] https://crrev.com/b88cae8f9b3af0b71bacda3dc187330d5c98fc07/chromeos-base/tast-cmd/tast-cmd-9999.ebuild

Comment 6 by derat@chromium.org, Feb 21 2018

Status: Fixed (was: Assigned)
https://crrev.com/c/923388 made tast_Runner log the version.

Sign in to add a comment