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

Issue 846369 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

elm-paladin failure in BuildPackages - error cloning from github.com/mafredri/cdp.git

Reported by jrbarnette@chromium.org, May 24 2018

Issue description

This CQ run failed:
    https://uberchromegw.corp.google.com/i/chromeos/builders/master-paladin/builds/18691

One of the failures was in elm-paladin:
    https://luci-milo.appspot.com/buildbot/chromeos/elm-paladin/6336

The error occurred during BuildPackages; the relevant output is below:

cdp-1.18.1: >>> Unpacking source...
cdp-1.18.1: From https://chromium.googlesource.com/external/github.com/mafredri/cdp
cdp-1.18.1:  * [new branch]      master     -> origin/master
cdp-1.18.1: GIT update -->
cdp-1.18.1:    repository:               https://chromium.googlesource.com/external/github.com/mafredri/cdp.git
cdp-1.18.1:    at the commit:            52b8ab24b49848d8f54145760f7a5eb122130e70
cdp-1.18.1:    commit:                   v1.18.1
cdp-1.18.1:    branch:                   master
cdp-1.18.1:    storage directory:        "/var/cache/chromeos-cache/distfiles/target/egit-src/external/github.com/mafredri/cdp"
cdp-1.18.1:    checkout type:            bare repository
cdp-1.18.1: Cloning into '/build/elm/tmp/portage/dev-go/cdp-1.18.1/work/cdp-1.18.1/src/github.com/mafredri/cdp'...
cdp-1.18.1: done.
cdp-1.18.1: fatal: 'v1.18.1' is not a commit and a branch 'tree-v1.18.1' cannot be created from it
cdp-1.18.1:  * ERROR: dev-go/cdp-1.18.1::chromiumos failed (unpack phase):

We have two problems here:
 1) The build failed, possibly because of a change in the upstream
    repository.
 2) Apparently, the build depends on a github repository.

As a rule, depending on external repositories for Chrome OS builds is
frowned on, precisely because it can result in this kind of failure.

 
Looking back through previous runs, this package has built successfully:

Completed dev-go/cdp-1.18.1 (in 3m17.8s)

The package has been updated in the past two weeks but one would assume it would have showed up prior to now.  The changes related to that run were not impacted by the CL in question.  That said, the build is failing in the subsequent execution as well for the same package.

Looking upstream there was a commit to cdp this morning:  https://github.com/mafredri/cdp/commit/52b8ab24b49848d8f54145760f7a5eb122130e70

-- Mike

> The package has been updated in the past two weeks but one
> would assume it would have showed up prior to now [ ... ]

I think the key to the error is this message:
> cdp-1.18.1: fatal: 'v1.18.1' is not a commit and a branch 'tree-v1.18.1' cannot be created from it

That suggests that there used to be a commit know by the name "v1.18.1"
in the tree, but that the name no longer exists.  That probably wasn't
caused by a commit, but by some other administrative change.

Changes of this sort can and do cause breakage downstream.  To avoid that,
the practice has been that our builds don't depend on external repositories
directly.  Instead, we require a mirror that gets updated automatically,
and we build from the mirror.

Status: Started (was: Untriaged)
Thanks for the guidance, Richard.  

I'm building locally to validate and whether we should roll back https://chromium.git.corp.google.com/chromiumos/overlays/chromiumos-overlay/+/a66b67bfc0d3c7c8e87afbe0d48d59abc533ca20%5E%21/#F1, which upgrades the package to 1.8.1.  

Will update as soon as I have additional answers.

-- Mike
One option is to mark elm as experimental right now and assign this to derat@ to evaluate upgrading to v1.8.2 and switching the git  hash instead of a tag, if this is the way this git repo is managed.

The CDP maintainer should probably be warned about deleting git tags, too.
The full impact isn't known, whether it is only elm or not.  I'm going to validate and then test the rollback.  Will work with derat@ (appears OOO on an outing) to get this resolved.

-- Mike
One good way to evaluate impact is to check the canaries; if this
is widespread, the canaries will say so.

I just checked the canaries for today, since the upstream change was made earlier today, and there are no other failures related to this package.  Oddly enough I cannot recreate these locally; build_packages executed successfully for elm.

I'm going to continue to poke at it.

-- Mike

I am not seeing this as a widespread issue, so to provide a bit of time to validate further and revert if needed, I'm going to go ahead and mark Elm as experimental for now.

-- Mike

Comment 9 by derat@chromium.org, May 24 2018

Cc: rahulchaudhry@chromium.org vapier@chromium.org derat@chromium.org
Labels: OS-Chrome
Sigh. When I updated this package, I thought it was odd that it jumped from v0.17.4 to v1.18.1, but hey, semantic versioning. :-/ Github (and our googlesource.com mirror) just show v0.18.whatever tags now, so I'm guessing that the repo owner typoed the version initially and deleted the v1.18.1 tag later.

Rahul or Mike, any ideas about the best way to recover from this?
> [ ... ] Github (and our googlesource.com mirror) just show v0.18.whatever [ ... ]

Oh, we have a mirror?  The logs made me think we're pulling directly
from github...

> cdp-1.18.1: GIT update -->
> cdp-1.18.1:    repository:               https://chromium.googlesource.com/external/github.com/mafredri/cdp.git

D'oh!  Reading that more carefully, that looks like a mirror to me...

Of course, either way, it looks like something upstream propagated
in to cause the failure.

When did we upgrade from v0.17.4?  Is it plausible to return to that
version right now while we sort things out?


Comment 12 by derat@chromium.org, May 24 2018

Status: Untriaged (was: Started)
I upgraded on May 11 with https://chromium-review.googlesource.com/1055937. There was no pressing need for the upgrade, and I suspect it'd be fine to return to v0.17.4 (or switch to v0.18.1) -- I'm just not sure of the build implications of doing so. What happens to chroots that have already built v1.18.1?
Fwiw, we're switching Go ebuilds to fetching tarballs (from chromeos-localmirror) instead of cloning git repositories (from chromium.googlesource.com): https://chromium-review.googlesource.com/1062490

This issue will get fixed as a side effect of that CL, since we have a "github.com-mafredri-cdp-v1.18.1.tar.gz" on our chromeos-localmirror. It was fetched when the tag still existed on the upstream github repository.

> [ ... ] What happens to chroots that have already built v1.18.1?

My first answer to that would be "we should sort that out after
elm-paladin is passing again."

But the "sort it later" idea may not be an issue:  I think if we jsut
apply an ebuild change that reverts to 0.17.4 and increases the ebuild
version number; then new chroots would just build the old code.
Re comment #13:  Yeah, that should fix the problem long-term.
We still need a short term fix better than
"EXPERIMENTAL=elm-paladin  crbug.com/846369 ".

Comment 16 by derat@chromium.org, May 24 2018

#14: What do you mean by "reverts to 0.17.4 and increases the ebuild version number"? Hardcode CROS_WORKON_COMMIT="v0.17.4" in the ebuild but rename the file to cdp-X.ebuild, where X is greater than 1.18.1?
> Hardcode CROS_WORKON_COMMIT="v0.17.4" in the ebuild but rename the file to cdp-X.ebuild, where X is greater than 1.18.1?

Oh.  I suppose I should read the code before I blithely suggest changes...

You're right, we might have to do portage magic of some sort.  I have some
idea that masking the problem version might be the mechanism?

> You're right, we might have to do portage magic of some sort.  I have some
> idea that masking the problem version might be the mechanism?

Also, whatever the magic is, we're going to have to do it:  It sounds like
long term, the version formerly known as "v1.18.1" shouldn't be used, since
the next version will be lower.

Comment 19 by derat@chromium.org, May 24 2018

I uploaded https://crrev.com/c/1072897, which just does a simple rename of the 1.18.1 ebuild to 0.18.1, to at least start some discussion.
Many other paladin builders failed. Not sure will the CL in comment #19 solves this build. Just filed a trybot to try that.

https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?buildbucketId=8945567633996560368
ideally we'd have a way of restoring the tag on our mirror ...
Project Member

Comment 22 by bugdroid1@chromium.org, May 25 2018

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

commit c8f34fdecd3f4d492732ccd878cd37e377362263
Author: Daniel Erat <derat@chromium.org>
Date: Fri May 25 09:43:03 2018

dev-go/cdp: Rename 1.18.1 to 0.18.1.

It looks like there was a mistake upstream that resulted in
a v1.18.1 tag being created in place of v0.18.1.
a66b67bfc0d3 updated our ebuild to the "new" version. The
v1.18.1 tag no longer exists in the upstream repo (or in our
mirror of it), so switch to the v0.18.1 tag.

BUG= chromium:846369 
TEST=none

Change-Id: Ibe11c1565f2da8a20dd83c946755a5a36bd008f0
Reviewed-on: https://chromium-review.googlesource.com/1072897
Tested-by: Dan Erat <derat@chromium.org>
Trybot-Ready: Dan Erat <derat@chromium.org>
Reviewed-by: Shen-En Shih <petershih@chromium.org>
Commit-Queue: Shen-En Shih <petershih@chromium.org>

[rename] https://crrev.com/c8f34fdecd3f4d492732ccd878cd37e377362263/dev-go/cdp/cdp-0.18.1.ebuild

Owner: mikenichols@chromium.org
Status: Started (was: Untriaged)
Status: Fixed (was: Started)
This should now be fixed.  The issue was resolved locally and the package was updated per upstream.  

-- Mike
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 13

Labels: merge-merged-factory-nami-10715.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/6959f2f0816f58e73480466623e64b615f976690

commit 6959f2f0816f58e73480466623e64b615f976690
Author: Daniel Erat <derat@chromium.org>
Date: Fri Jul 13 14:43:53 2018

dev-go/cdp: Rename 1.18.1 to 0.18.1.

It looks like there was a mistake upstream that resulted in
a v1.18.1 tag being created in place of v0.18.1.
a66b67bfc0d3 updated our ebuild to the "new" version. The
v1.18.1 tag no longer exists in the upstream repo (or in our
mirror of it), so switch to the v0.18.1 tag.

BUG= chromium:846369 
TEST=none

Change-Id: Ibe11c1565f2da8a20dd83c946755a5a36bd008f0
Reviewed-on: https://chromium-review.googlesource.com/1072897
Tested-by: Dan Erat <derat@chromium.org>
Trybot-Ready: Dan Erat <derat@chromium.org>
Reviewed-by: Shen-En Shih <petershih@chromium.org>
Commit-Queue: Shen-En Shih <petershih@chromium.org>
(cherry picked from commit c8f34fdecd3f4d492732ccd878cd37e377362263)
Reviewed-on: https://chromium-review.googlesource.com/1136411
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Zhuohao Lee <zhuohao@google.com>
Tested-by: Zhuohao Lee <zhuohao@google.com>

[rename] https://crrev.com/6959f2f0816f58e73480466623e64b615f976690/dev-go/cdp/cdp-0.18.1.ebuild

Sign in to add a comment