go ebuild guide should not tell people to use cros-workon |
||||||||
Issue descriptionthe cros-workon eclass should be largely reserved for repos that are hosted on our GoB infrastructure. using them to fetch code from external sites (like github) is bad and pointless: it wastes a lot of bandwidth doing git which slows everything down, and it can simply be switched to fetching a tarball from github's infra.
,
Aug 1 2016
sorry, it looks like i read things hastily while trying to make it through my backlog. let me try to make up for it with some constructive thoughts ... instead of suggesting people use git hashes, should we push for using tagged releases ? howeyc/fsnotify for example has a number of them. i tend to feel a lot better when i see something like: CROS_WORKON_COMMIT="v0.9.0" rather than: CROS_WORKON_COMMIT="some-long-git-hash-arbitrarily-chosen-with-no-promise-of-QA" i'm not sure the CROS_WORKON_TREE setting is clear. for someone who has never looked at this before, how do they know how to compute this ? i guess we just need to extend the info in the cros-workon.eclass comments and add an example there ?
,
Aug 1 2016
CROS_WORKON_TREE is the hash of the git log specified by CROS_WORKON_COMMIT, it can be computed by "git log -1 --format=%T" if it's TOT, or "git log -1 --format=%T <hash_id>". Link at http://dev.chromium.org/chromium-os/chromiumos-design-docs/verify-prebuilts-using-content-hashing
,
Aug 1 2016
actually, on chromium.googlesource.com/external/github.com, the "tree" value is calculated for each CL. example of this convenience: https://chromium.googlesource.com/external/github.com/howeyc/fsnotify/+/f0c08ee9c60704c1879025f2ae0ff3e000082c13
,
Aug 1 2016
Regarding vendoring/versioning of Go packages, the current state is unfortunately not very good. The version numbers you see for fsnotify package are the exception rather than the norm. Most third party Go packages on github.com/ or even on go.googlesource.com/x/ don't have version tags, and the "go get" command simply fetches the source from the tip-of-master-branch. There's also the confusion about how to specify version numbers even in the cases they're present. Should the version be in a CROS_WORKON_COMMIT="v0.9.0" as you suggested, or should the version number be in the ebuild name (fsnotify-0.9.0.ebuild) like the Portage convention, and if it's the latter, we need extra machinery to automatically checkout the correct tag from a cloned git repository (which is probably some new logic that is not really specific to Go). As a compromise, we can have a convention that I suggested on the fsnotify cl: CROS_WORKON_COMMIT="441bbc86b167f3c1f4786afae9931403b99fdacf" # v0.9.0 CROS_WORKON_TREE="3038f8900106bc2b4c46b9c93f3de17a558d03b6" where are hashes are from https://chromium.googlesource.com/external/github.com/howeyc/fsnotify/+/v0.9.0 i.e. if version tags are available in the repository, use the commit/tree hashes from one of the versioned releases, and specify the version in a comment.
,
Aug 2 2016
i don't think extra work is needed here. the ebuild should reflect the tag, and then we can simply use the portage variables.
so we use fsnotify-0.9.0.ebuild and then the ebuild does:
CROS_WORKON_COMMIT="v${PV}"
that's all there is to it :).
,
Aug 2 2016
Tested by co-existing multiple ebuild in same folder. emerge can always find the maximum versioned file to run. For example, fsnotify/fsnotify right now is at v1.3.1. One way is to create fsnotify-1.3.1.ebuild, and create fsnotify-1.3.1-r#.ebuild for every incremental change, before next version tagged. Creating 0.0.1.ebuild has its own benefit. For example for dev-go/dbus, it changed to 0.0.2 due to underlying package change: like below log: > dev-go/dbus: use github.com/godbus/dbus as the source repository > > Previously used guelfey/go.dbus is dead/unmaintained
,
Aug 2 2016
here's a test on go-fsnotify/fsnotify on chromium mirror. Right now TOT is at 1.2.9. CROS_WORKON_COMMIT="8611c35ab31c1c28aa903d33cf8b6e44a399b09e" # v1.2.9 CROS_WORKON_TREE="b590050905bc53ee3f51bff232358f1973ea6f3e" >>> Unpacking source... GIT update --> repository: https://chromium.googlesource.com/external/github.com/go-fsnotify/fsnotify.git at the commit: 8611c35ab31c1c28aa903d33cf8b6e44a399b09e commit: 8611c35ab31c1c28aa903d33cf8b6e44a399b09e branch: master storage directory: "/var/cache/chromeos-cache/distfiles/target/egit-src/external/github.com/go-fsnotify/fsnotify" checkout type: bare repository Cloning into '/build/whirlwind/tmp/portage/dev-go/fsnotify-1.2.9/work/fsnotify-1.2.9/src/github.com/go-fsnotify/fsnotify'... done. Switched to a new branch 'tree-8611c35ab31c1c28aa903d33cf8b6e44a399b09e' When I use SHA for 1.28, please see below log. CROS_WORKON_COMMIT="508915b7500b6e42a87132e4afeb4729cebc7cbb" # v1.2.8 CROS_WORKON_TREE="792ef2dd9ca1ff73365bf48beec1bb1289418227" >>> Unpacking source... GIT update --> repository: https://chromium.googlesource.com/external/github.com/go-fsnotify/fsnotify.git at the commit: 8611c35ab31c1c28aa903d33cf8b6e44a399b09e commit: 508915b7500b6e42a87132e4afeb4729cebc7cbb branch: master storage directory: "/var/cache/chromeos-cache/distfiles/target/egit-src/external/github.com/go-fsnotify/fsnotify" checkout type: bare repository Cloning into '/build/whirlwind/tmp/portage/dev-go/fsnotify-1.2.8/work/fsnotify-1.2.8/src/github.com/go-fsnotify/fsnotify'... done. Switched to a new branch 'tree-508915b7500b6e42a87132e4afeb4729cebc7cbb' Looks like emerge is able to switch to correct commit specified by CROS_WORKON_COMMIT
,
Aug 2 2016
Great if CROS_WORKON_COMMIT="v${PV}" just works.
Note that this implies not specifying CROS_WORKON_TREE at all in the ebuild. I'm assuming cros-workon/git eclasses are ok with that.
Zhihong, did you try Mike's suggestion with naming the ebuild fsnotify-1.2.9.ebuild and specifying
CROS_WORKON_COMMIT="v${PV}"
inside the ebuild, and removing the CROS_WORKON_TREE setting altogether?
If this works, let's use that scheme for this package.
And yes, for every incremental change to the ebuild, you'd use fsnotify-1.2.9-r#.ebuild, until the upgrade to the next revision tag.
,
Aug 2 2016
i don't think it'll be a prob to leave CROS_WORKON_TREE blank. the issue that var was created to mitigate doesn't exist with tagged versions.
,
Aug 3 2016
for fsnotify, using CROS_WORKON_COMMIT="${PV}" works. Commented out CROS_WORKON_TREE or leave it blank still works, with the same log.
From the description of adding CROS_WORKON_TREE, will removing it affecting prebuilt? I'm not clear what it's for.
>>> Emerging (1 of 1) dev-go/fsnotify-1.2.9::chromiumos for /build/whirlwind/
* Running stacked hooks for pre_pkg_setup
* sysroot_build_bin_dir ... [ ok ]
* Running stacked hooks for pre_src_unpack
* python_multilib_setup ... [ ok ]
>>> Unpacking source...
GIT update -->
repository: https://chromium.googlesource.com/external/github.com/go-fsnotify/fsnotify.git
at the commit: 8611c35ab31c1c28aa903d33cf8b6e44a399b09e
commit: v1.2.9
branch: master
storage directory: "/var/cache/chromeos-cache/distfiles/target/egit-src/external/github.com/go-fsnotify/fsnotify"
checkout type: bare repository
Cloning into '/build/whirlwind/tmp/portage/dev-go/fsnotify-1.2.9/work/fsnotify-1.2.9/src/github.com/go-fsnotify/fsnotify'...
done.
Switched to a new branch 'tree-v1.2.9'
>>> Emerging (1 of 1) dev-go/fsnotify-1.2.8::chromiumos for /build/whirlwind/
* Running stacked hooks for pre_pkg_setup
* sysroot_build_bin_dir ... [ ok ]
* Running stacked hooks for pre_src_unpack
* python_multilib_setup ... [ ok ]
>>> Unpacking source...
GIT update -->
repository: https://chromium.googlesource.com/external/github.com/go-fsnotify/fsnotify.git
at the commit: 8611c35ab31c1c28aa903d33cf8b6e44a399b09e
commit: v1.2.8
branch: master
storage directory: "/var/cache/chromeos-cache/distfiles/target/egit-src/external/github.com/go-fsnotify/fsnotify"
checkout type: bare repository
Cloning into '/build/whirlwind/tmp/portage/dev-go/fsnotify-1.2.8/work/fsnotify-1.2.8/src/github.com/go-fsnotify/fsnotify'...
done.
Switched to a new branch 'tree-v1.2.8'
,
Aug 3 2016
This page describes why CROS_WORKON_TREE was added: http://www.chromium.org/chromium-os/chromiumos-design-docs/verify-prebuilts-using-content-hashing I don't think this affects ebuilds that import external Go packages.
,
Aug 3 2016
I'll need to add a dependency to fsnotify as it depends on "dev-go/go-sys". Shall I use DEPEND or RDEPEND? Searched around and looks like all ebuild go dependencies in the code base are in DEPEND. Is this because the go package is imported as source rather than library?
,
Aug 3 2016
In this case, you should use RDEPEND. This is because fsnotify does not build a binary, it only exports a Go package. The exported Go package will work only as long as the package from dev-go/go-sys is also installed. This is ensured by RDEPEND. This is explained in the dev-guide, although perhaps it is not very clear: http://www.chromium.org/chromium-os/developer-guide/go-in-chromium-os -) If a dependency package is needed to build the executables, put it in DEPEND. -) If a dependency package is imported by the Go packages installed by this ebuild, put it in RDEPEND.
,
Aug 3 2016
Thanks! Found in the link, many notations on DEPEND/RDEPEND.
,
Aug 15 2016
so to circle back, we want to update the guide to cover the case of projects that use tags/releases and expand on the TREE variable right ?
,
Aug 15 2016
Sounds good.
,
Aug 15 2016
i assume you'll write those updates ? :)
,
Aug 15 2016
Yes. I changed the status to Assigned in previous reply, and I'm the owner already :)
,
Aug 15 2016
odd ... my e-mail & bug view didn't include that Status label change
,
Oct 27 2016
Updated http://www.chromium.org/chromium-os/developer-guide/go-in-chromium-os The first ebuild example is now dev-go/fsnotify, which uses ebuild version to fetch and install the corresponding tag. Removed all usage of CROS_WORKON_TREE from dev-go/*: https://chromium-review.googlesource.com/#/c/403520 Updated all ebuild examples on the wiki page. Also added some missing documentation (variable CROS_GO_TEST).
,
Oct 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/223eafbf01c9bcd8646f386bda1909d70fe0a9ed commit 223eafbf01c9bcd8646f386bda1909d70fe0a9ed Author: Rahul Chaudhry <rahulchaudhry@chromium.org> Date: Wed Oct 26 23:09:15 2016 dev-go: remove redundant CROS_WORKON_TREE specifications. Specifying CROS_WORKON_TREE is not required in these ebuilds. It is uniquely determined by CROS_WORKON_COMMIT. Also cleaned up some DESCRIPTION and HOMEPAGE values. BUG= chromium:633101 TEST='sudo emerge dev-go/{dbus,fsnotify,glog,go-sys,go-tools,golint,mock,net,protobuf}' works. Change-Id: I796fb4f8a74d47598b415d9892ca0465c5ddb668 Reviewed-on: https://chromium-review.googlesource.com/403520 Commit-Ready: Rahul Chaudhry <rahulchaudhry@chromium.org> Tested-by: Rahul Chaudhry <rahulchaudhry@chromium.org> Reviewed-by: Caroline Tice <cmtice@chromium.org> [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/go-sys/go-sys-0.0.1.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/dbus/dbus-0.0.2.ebuild [rename] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/golint/golint-0.0.1-r2.ebuild [add] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/fsnotify/fsnotify-1.3.1-r1.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/seccomp/seccomp-9999.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/mock/mock-0.0.1.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/protobuf/protobuf-0.0.1.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/net/net-0.0.1.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/fsnotify/fsnotify-1.3.1.ebuild [rename] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/glog/glog-0.0.1-r3.ebuild [rename] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/protobuf/protobuf-0.0.1-r6.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/golint/golint-0.0.1.ebuild [add] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/go-sys/go-sys-0.0.1-r1.ebuild [rename] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/net/net-0.0.1-r3.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/glog/glog-0.0.1.ebuild [rename] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/dbus/dbus-0.0.2-r4.ebuild [modify] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/go-tools/go-tools-0.0.1.ebuild [rename] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/go-tools/go-tools-0.0.1-r3.ebuild [rename] https://crrev.com/223eafbf01c9bcd8646f386bda1909d70fe0a9ed/dev-go/mock/mock-0.0.1-r2.ebuild
,
Oct 31 2016
,
Jan 21 2017
,
Mar 4 2017
,
Apr 17 2017
,
May 30 2017
,
Aug 1 2017
,
Oct 14 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by rahulchaudhry@chromium.org
, Aug 1 2016