Issue 717502  has been merged into this issue.

Locally running the recipe confirms the theory. Raising the prio as it is quite harmful that an empty "Bug: " footer can bring _all_ buildbots down.

work around for depot_tools: https://chromium-review.googlesource.com/493288

ehr, wrong link

https://chromium-review.googlesource.com/493149 is the work around

I thought I fixed the Foo: bad parsing in git_footers & CQ last week. Looking deeper now...

Ah, crap. We have footers parsing in bot_update. Let's fix that instead.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/e0a1afbf4b0d41b10d9eb1463835647024eb25bc

commit e0a1afbf4b0d41b10d9eb1463835647024eb25bc
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Tue May 02 14:39:49 2017

Fix bot_update parsing of gerrit footers with empty values.

Follow up based on https://chromium-review.googlesource.com/487963

R=agable@chromium.org, machenbach@chromium.org, maruel@chromium.org

Bug: 715614, 717504 
Change-Id: I0902647c8fa8a2ff9643890c029fc8b75a72ff7e
Reviewed-on: https://chromium-review.googlesource.com/493466
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>

[modify] https://crrev.com/e0a1afbf4b0d41b10d9eb1463835647024eb25bc/recipes/recipe_modules/bot_update/resources/bot_update.py

This Cl has been rolled to build. I've retriggered the same builder on the same revision and things went well:

https://uberchromegw.corp.google.com/i/client.v8/builders/V8%20Linux%20-%20builder/builds/25034/steps/bot_update/logs/stdio

So, marking as fixed.

This particular bug is fixed, but I think when manually committing it's still possible to knock out everything with a wrong footer. E.g. by slipping an invalid line into the footer block:

Bug: 
TBR=hax
Change-Id: Ib6d3768a9e0cfb9ea5fdcece25325e8d498520af

Line 2 will not match, and as a result bot_update's footer map will be empty.

How about we read the footers line by line from the bottom and return the footers we have on the first invalid footer instead of invalidating all? The machine-added footers are always on the bottom and should always be correct. It shouldn't be possible to invalidate them with user-added lines.

I agree with above, as I think that's indeed possible. I don't think I like parsing footers till last malformed footer line though. I'd rather make everything in infra parse Gerrit footers the way gerrit does.

As of now, Gerrit considers footers whatever is in the last paragraph, and Gerrit just ignores malformed footer lines. Thus, for Gerrit given commit message = """
Title

Footer: OK
(whatever, it's ignored)
Still-Footer: OK
"""

has two footers. At the same time, git_footers.py library, its fork in CQ, and similar functionality in bot_update are more stringent.

So, for bot_update, we could just turn https://chromium.googlesource.com/chromium/tools/depot_tools/+/86989d7b0f709d43a4f48b8e7a17a111576c96fc/recipes/recipe_modules/bot_update/resources/bot_update.py#415 into a "continue" and that's it.

It's nice to align with gerrit, however, I don't think it should be possible to kill the entire waterfall with a poorly formatted commit message..

That's the point of tandrii@'s comment; if we align with gerrit, we'd be much more permissive and not kill the waterfall when someone commits bad footers.

tandrii, I agree with your assessment. Let's make all git-footers parsing in infra match the way JGit does it.

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/585428b8a1a8edcafb68c3a85eef5ae88e244054

commit 585428b8a1a8edcafb68c3a85eef5ae88e244054
Author: Michael Achenbach <machenbach@chromium.org>
Date: Wed May 10 09:49:56 2017

[release] Fix parsing of gerrit footers with empty values.

This ports:
https://chromium-review.googlesource.com/c/493466/

The code was originally copied from bot_update. So were the bugs.

Bug:  chromium:717504 
NOTRY=true
TBR=tandrii@chromium.org,agable@chromium.org

Change-Id: If2d2dafdca8cd44f325dc770dfc42c17889a3b4a
Reviewed-on: https://chromium-review.googlesource.com/501787
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45224}
[modify] https://crrev.com/585428b8a1a8edcafb68c3a85eef5ae88e244054/tools/release/git_recipes.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/28a5d5defd13c382af94cd8368b16641752fee88

commit 28a5d5defd13c382af94cd8368b16641752fee88
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed May 10 17:44:03 2017

Relax git_footers parsing to match that of Gerrit (JGit).

R=agable@chromium.org

Bug:  717504 
Change-Id: Ieb6415d55e85b91f11f9052b0fd08cf982b64d51
Reviewed-on: https://chromium-review.googlesource.com/501849
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/28a5d5defd13c382af94cd8368b16641752fee88/recipes/recipe_modules/bot_update/resources/bot_update.py
[modify] https://crrev.com/28a5d5defd13c382af94cd8368b16641752fee88/git_footers.py
[modify] https://crrev.com/28a5d5defd13c382af94cd8368b16641752fee88/tests/git_footers_test.py

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/3832db0ed1a9d8cd8ccaa000788d1e38e2ac2856

commit 3832db0ed1a9d8cd8ccaa000788d1e38e2ac2856
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed May 10 18:36:18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a6424c7626bceca3dde53f88a6a7c3fea7387bdf

commit a6424c7626bceca3dde53f88a6a7c3fea7387bdf
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu May 11 09:02:47 2017

[release] Relax git_footers parsing to match that of Gerrit (JGit).

Port https://chromium-review.googlesource.com/c/501849/

NOTRY=true
TBR=tandrii@chromium.org
Bug:  chromium:717504 

Change-Id: Ia37759c615cc3ad4d2978a4589ca687a750afc46
Reviewed-on: https://chromium-review.googlesource.com/503028
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45249}
[modify] https://crrev.com/a6424c7626bceca3dde53f88a6a7c3fea7387bdf/tools/release/git_recipes.py

Getting this on CL upload now:
machenbach@malumi:~/v8/v8 gcc2134 $ git up
Using 50% similarity for rename/copy detection. Override with --similarity.
 infra/config/cq.cfg | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
Traceback (most recent call last):
  File "/usr/local/google/home/machenbach/tools/depot_tools/git_cl.py", line 5932, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/usr/local/google/home/machenbach/tools/depot_tools/git_cl.py", line 5914, in main
    return dispatcher.execute(OptionParser(), argv)
  File "/usr/local/google/home/machenbach/tools/depot_tools/subcommand.py", line 252, in execute
    return command(parser, args[1:])
  File "/usr/local/google/home/machenbach/tools/depot_tools/git_cl.py", line 4808, in CMDupload
    return cl.CMDUpload(options, args, orig_args)
  File "/usr/local/google/home/machenbach/tools/depot_tools/git_cl.py", line 1644, in CMDUpload
    ret = self.CMDUploadChange(options, git_diff_args, custom_cl_base, change)
  File "/usr/local/google/home/machenbach/tools/depot_tools/git_cl.py", line 2894, in CMDUploadChange
    GenerateGerritChangeId(change_desc.description)))
  File "/usr/local/google/home/machenbach/tools/depot_tools/git_footers.py", line 86, in add_footer_change_id
    after_keys=['Bug', 'Issue', 'Test', 'Feature'])
  File "/usr/local/google/home/machenbach/tools/depot_tools/git_footers.py", line 116, in add_footer
    if normalize_name(parse_footer(x)[0]) == k]
TypeError: 'NoneType' object has no attribute '__getitem__'

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/43ec62ec712c3f199f85c8abec6db77a9e5e81e1

commit 43ec62ec712c3f199f85c8abec6db77a9e5e81e1
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu May 11 09:54:31 2017

Revert "Relax git_footers parsing to match that of Gerrit (JGit)."

This reverts commit 28a5d5defd13c382af94cd8368b16641752fee88.

Reason for revert: breaks assumption in function for adding footers

Original change's description:
> Relax git_footers parsing to match that of Gerrit (JGit).
> 
> R=​agable@chromium.org
> 
> Bug:  717504 
> Change-Id: Ieb6415d55e85b91f11f9052b0fd08cf982b64d51
> Reviewed-on: https://chromium-review.googlesource.com/501849
> Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
> Reviewed-by: Aaron Gable <agable@chromium.org>
> 

TBR=agable@chromium.org,machenbach@chromium.org,tandrii@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Bug:  717504 

Change-Id: I9b4d619b2972be8434aff9464f1959fbcb3abd32
Reviewed-on: https://chromium-review.googlesource.com/503030
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/43ec62ec712c3f199f85c8abec6db77a9e5e81e1/recipes/recipe_modules/bot_update/resources/bot_update.py
[modify] https://crrev.com/43ec62ec712c3f199f85c8abec6db77a9e5e81e1/git_footers.py
[modify] https://crrev.com/43ec62ec712c3f199f85c8abec6db77a9e5e81e1/tests/git_footers_test.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/43ec62ec712c3f199f85c8abec6db77a9e5e81e1

commit 43ec62ec712c3f199f85c8abec6db77a9e5e81e1
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu May 11 09:54:31 2017

Revert "Relax git_footers parsing to match that of Gerrit (JGit)."

This reverts commit 28a5d5defd13c382af94cd8368b16641752fee88.

Reason for revert: breaks assumption in function for adding footers

Original change's description:
> Relax git_footers parsing to match that of Gerrit (JGit).
> 
> R=​agable@chromium.org
> 
> Bug:  717504 
> Change-Id: Ieb6415d55e85b91f11f9052b0fd08cf982b64d51
> Reviewed-on: https://chromium-review.googlesource.com/501849
> Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
> Reviewed-by: Aaron Gable <agable@chromium.org>
> 

TBR=agable@chromium.org,machenbach@chromium.org,tandrii@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Bug:  717504 

Change-Id: I9b4d619b2972be8434aff9464f1959fbcb3abd32
Reviewed-on: https://chromium-review.googlesource.com/503030
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/43ec62ec712c3f199f85c8abec6db77a9e5e81e1/recipes/recipe_modules/bot_update/resources/bot_update.py
[modify] https://crrev.com/43ec62ec712c3f199f85c8abec6db77a9e5e81e1/git_footers.py
[modify] https://crrev.com/43ec62ec712c3f199f85c8abec6db77a9e5e81e1/tests/git_footers_test.py

Tried to upload with this text:

[CQ] Make linux64 gcc debug bot mandatory

Bug:  v8:6355 
NOTRY=true
TBR=sergiyb@chromium.org

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/04b51d6de0c1eb267f2446126a682fc747620439

commit 04b51d6de0c1eb267f2446126a682fc747620439
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu May 11 12:27:14 2017

Reland of Relax git_footers parsing to match that of Gerrit (JGit).

Previously landed as:
> Change-Id: Ieb6415d55e85b91f11f9052b0fd08cf982b64d51
> Reviewed-on: https://chromium-review.googlesource.com/501849

R=agable@chromium.org,machenbach@chromium.org

Bug:  717504 
Change-Id: Iede5c29ff4c317b68d8c2bca319edec140a176f5
Reviewed-on: https://chromium-review.googlesource.com/502908
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/04b51d6de0c1eb267f2446126a682fc747620439/recipes/recipe_modules/bot_update/resources/bot_update.py
[modify] https://crrev.com/04b51d6de0c1eb267f2446126a682fc747620439/git_footers.py
[modify] https://crrev.com/04b51d6de0c1eb267f2446126a682fc747620439/tests/git_footers_test.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/1a91c6080a92423c8816208ed0931bdcbdf05f2d

commit 1a91c6080a92423c8816208ed0931bdcbdf05f2d
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu May 11 12:49:26 2017

Nit fix for git_footers.

TBR=machenbach@chromium.org
Bug:  717504 
Change-Id: Ifa71fa4cac0b57ed11ba5e7eb93516b23b72a496
Reviewed-on: https://chromium-review.googlesource.com/502809
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/1a91c6080a92423c8816208ed0931bdcbdf05f2d/git_footers.py

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infra/infra_internal/+/156e7333a16bf0d58875756f147d86dca47d5cb5

commit 156e7333a16bf0d58875756f147d86dca47d5cb5
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Thu May 11 12:56:20 2017