New issue
Advanced search Search tips

Issue 639677 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature



Sign in to add a comment

Add PRESUBMITs or build rules to net/base/registry_controlled_domain

Project Member Reported by rsleevi@chromium.org, Aug 21 2016

Issue description

Whenever updating registry_controlled_domain.dat, it's necessary to re-run the "tld_cleanup" tool from /net/tools to generate the .gperf file, which is then used by make_dafsa.py to generate the final result.

However, it's possible to update the .dat file and forget to run tld_cleanup to generate the .gperf file; if this happens, the registry controlled domains won't actually be updated.

Ideally, we would make the .dat generate the .gperf directly with tld_cleanup as a build step, but if there's concern about the build time costs, we should alternatively add a PRESUBMIT.py to ensure that if a CL contains an update to the .dat or the .gperf, then the other file is also included in the CL.
 
Components: Internals>Network
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 21 2016

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

commit a16e791c295658f7253f30ab7680ce74b7e0b93b
Author: rsleevi <rsleevi@chromium.org>
Date: Sun Aug 21 21:43:33 2016

Update the generated public suffix file

Commit 702d65e434b5a41965f5b9fbe644e9e4300157b6
updated the underlying data file, but the
generated .gperf file was not re-generated because
I forgot to run net/tools/tld_cleanup on it.

TBR'ing because it's a file derived from the
already reviewed CL.

TBR=pkasting@chromium.org
BUG= 626532 , 639677

Review-Url: https://codereview.chromium.org/2263923002
Cr-Commit-Position: refs/heads/master@{#413380}

[modify] https://crrev.com/a16e791c295658f7253f30ab7680ce74b7e0b93b/net/base/registry_controlled_domains/effective_tld_names.gperf

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 25 2016

Labels: merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ec0afd588845ef505a04d3d4123949220fb18b8

commit 9ec0afd588845ef505a04d3d4123949220fb18b8
Author: Ryan Sleevi <rsleevi@chromium.org>
Date: Thu Aug 25 18:00:56 2016

Update the generated public suffix file

Commit 702d65e434b5a41965f5b9fbe644e9e4300157b6
updated the underlying data file, but the
generated .gperf file was not re-generated because
I forgot to run net/tools/tld_cleanup on it.

TBR'ing because it's a file derived from the
already reviewed CL.

TBR=pkasting@chromium.org
BUG= 626532 , 639677

Review-Url: https://codereview.chromium.org/2263923002
Cr-Commit-Position: refs/heads/master@{#413380}
(cherry picked from commit a16e791c295658f7253f30ab7680ce74b7e0b93b)

Review URL: https://codereview.chromium.org/2270193005 .

Cr-Commit-Position: refs/branch-heads/2785@{#750}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/9ec0afd588845ef505a04d3d4123949220fb18b8/net/base/registry_controlled_domains/effective_tld_names.gperf

Project Member

Comment 4 by sheriffbot@chromium.org, Aug 28 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by mmenke@chromium.org, Aug 29 2017

Status: Available (was: Untriaged)
Punting out of triage queue.  Assume this is still needed.

Comment 6 by mmenke@chromium.org, May 16 2018

Labels: Network-Triaged
[rsleevi]: I assume we don't have a better label for this?
Not unless we want to create >Base or >Core, but seems like that'd be more confusion for folks :)

Sign in to add a comment