Assigning to jbudorick@ since this looks similar to bug 855827

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/273d9d30a58df0a2f008ad52eaf90f88ecf87c52

commit 273d9d30a58df0a2f008ad52eaf90f88ecf87c52
Author: Michael Moss <mmoss@google.com>
Date: Mon Jun 25 22:57:49 2018

Nope, this is because of a change in the internal 3396 buildspec (around release .96), which was causing the internal variable to accidentally propagate to the public releases. #2 should prevent that for any subsequent releases, but I think we'll have to force the tarball creation for .99.

Note, the value that should be substituted for {chrome_git} is: 
https://chrome-internal.googlesource.com

I don't think forcing the tarball to be created is necessary, as the bot will keep trying to build that tag until it sees a tarball in Google Storage. The tag does need to be pushed again though, as the DEPS file is still referencing {chrome_git} without defining it.

It's generally not a good idea to repush tags, and wouldn't necessarily fix the tarball build anyhow, since once tags are fetched, git won't try to update them (since they're meant to be immutable). The best fix here would be to have the tarball script explicitly define the missing var (maybe just temporarily) in it's gclient config, like:

"custom_vars": { "chrome_git": "https://chrome-internal.googlesource.com" },

Hmm, that can also work but I'll only be able to work on it tomorrow (unless thomasanderson@ beats me to it). I mentioned re-pushing the tag because that's what was done last time ( bug 815124 ).

IIRC, that fix caused some problems in other places that expect the immutable nature of tags. I'm not completely opposed to repushing tags, but I think it should be reserved for extreme failures which don't have a relatively simple workaround, but that's not the case here.

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/1049288455ea4666f184d6ddda7c91919ba98a5f

commit 1049288455ea4666f184d6ddda7c91919ba98a5f
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Thu Jun 28 21:59:45 2018

publish_tarball: Explicitly set |chrome_git| for 67.0.3396.99.

This tag's DEPS file references |chrome_git| but does not define the
variable. Explicitly set it as a custom variable in gclient in order to
allow the checkout to properly proceed.

Bug:  856344 
Change-Id: I6f7bd0b2b0fbb5819675f15cdeb9f4295b279c5c
Reviewed-on: https://chromium-review.googlesource.com/1117681
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Michael Moss <mmoss@chromium.org>
Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>

[add] https://crrev.com/1049288455ea4666f184d6ddda7c91919ba98a5f/recipes/recipes/publish_tarball.expected/extra_config_params.json
[modify] https://crrev.com/1049288455ea4666f184d6ddda7c91919ba98a5f/recipes/README.recipes.md
[modify] https://crrev.com/1049288455ea4666f184d6ddda7c91919ba98a5f/recipes/recipes/publish_tarball.py

Looks like slave10-c1 is down, so 67.0.3396.99 still isn't getting published :(
https://build.chromium.org/deprecated/chromium.infra.cron/buildslaves/slave10-c1

This issue should be fixed, though bug 859129 is still open. 

Reopening; according to https://ci.chromium.org/buildbot/chromium.infra.cron/publish_tarball/11560 my fix didn't work, as the DEPS file is parsed without taking |custom_vars| into consideration:

    KeyError: "chrome_git was used as a variable, but was not declared in the vars dict (file '/b/rr/tmpIQCPG9/w/src/DEPS', line 111)"

mmoss, is there any other option here besides just editing the DEPS file manually in publish_tarball.py?

One solution I've been playing with is to have the bot call "gclient sync" with --patch-ref pointing to a CL patching DEPS (e.g. https://chromium-review.googlesource.com/c/chromium/src/+/1127176).

It's quite hackish, but if there's no intention to re-push the tags we'd need something along those lines, as the entire bot_update step is an atomic operation from publish_tarball.py's point of view (i.e. we cannot add a sed call replacing {chrome_git} with something else before gclient sync is called).

If everyone's OK, I can write a CL for that. I'd also need to upload and abandon CLs updating DEPS for 67.0.3396.101 and 67.0.3396.102 as well, since those versions were part of the cros-stable channel and will thus cause publish_tarball.py to attempt to create tarballs for them as well.

It looks like the custom_vars is getting through to the gclient command properly:

Loaded .gclient config in /b/rr/tmpIQCPG9/w:
solutions = [   {   'custom_vars': {   'checkout_telemetry_dependencies': 'True', 'chrome_git': 'https://chrome-internal.googlesource.com'},
        'deps_file': '.DEPS.git',
        'managed': False,
        'name': 'src',
        'url': 'https://chromium.googlesource.com/chromium/src.git'}]


I'm not sure, but it might not be having any effect because the 'src' checkout is marked as "managed: False". There's probably no reason why it should be that way on a non-interactive bot (and it's arguably incorrect that it is), so maybe we should just try setting that to "True" before applying any other hacks?

Would that matter if --revision src@refs/tags/67.0.3396.101 is being passed to bot_update?

Ah, yeah, with that, managed mode shouldn't be needed for fetching, but I wonder if it's still needed to apply the custom settings properly. I'm just guessing here, but I don't see any other reason why custom_vars wouldn't be applied. I'll see if I can reproduce that behavior with a local checkout.

I think the problem with custom_vars is that they only override variables that are set in DEPS; in this case we're trying to introduce a new one and parsing fails.

Oh, that may be. ISTR that was a discussion about that recently, and I thought we added support for "new" vars, but we might have decided it would never be needed :( +ehmaldonado might know off hand.

Yes we it override variables already declared in DEPS.
I don't recall supporting new vars being proposed, though...

With this in mind, should we pursue an alternative that doesn't involve custom_vars?

Since there doesn't appear to be a good workaround, I updated the tag:

https://chromium.googlesource.com/chromium/src.git/+/8ef023c8bf48f152812e2068fc21827bc5503917

Hopefully that won't confuse anything else.

My hero :)

Would it be possible to do the same to .101 and .102? They're in the cros-stable channel, so the bots are also trying and failing to build tarballs for them.

I haven't noticed any issues with the .99 change, so I did the same for 101 and 102.

Awesome, thanks a lot! Looking at https://ci.chromium.org/buildbot/chromium.infra.cron/publish_tarball/12214 I think everything's been fixed.

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/ff02941e8fe19be8b000652a26bc587e62a4d2f1

commit ff02941e8fe19be8b000652a26bc587e62a4d2f1
Author: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>
Date: Mon Jul 09 19:36:07 2018

Revert "publish_tarball: Explicitly set |chrome_git| for 67.0.3396.99."

This reverts commit 1049288455ea4666f184d6ddda7c91919ba98a5f.

Reason for revert: the workaround did not work, so let us just go back to what the code looked like before.

Original change's description:
> publish_tarball: Explicitly set |chrome_git| for 67.0.3396.99.
> 
> This tag's DEPS file references |chrome_git| but does not define the
> variable. Explicitly set it as a custom variable in gclient in order to
> allow the checkout to properly proceed.
> 
> Bug:  856344 
> Change-Id: I6f7bd0b2b0fbb5819675f15cdeb9f4295b279c5c
> Reviewed-on: https://chromium-review.googlesource.com/1117681
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
> Reviewed-by: Michael Moss <mmoss@chromium.org>
> Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>

TBR=agable@chromium.org,mmoss@chromium.org,raphael.kubo.da.costa@intel.com,tandrii@chromium.org,thomasanderson@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  856344 
Change-Id: Ic0092c00a3c19ff6083a6f47b4c4ff169d9f3a67
Reviewed-on: https://chromium-review.googlesource.com/1129228
Reviewed-by: Michael Moss <mmoss@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com>

[delete] https://crrev.com/c4be983061a342ce7e1c2e11e6bcc94112a8ddb9/recipes/recipes/publish_tarball.expected/extra_config_params.json
[modify] https://crrev.com/ff02941e8fe19be8b000652a26bc587e62a4d2f1/recipes/README.recipes.md
[modify] https://crrev.com/ff02941e8fe19be8b000652a26bc587e62a4d2f1/recipes/recipes/publish_tarball.py