ICU-10923 Fixing dependency graph and filter logic for collation.

- Fixes filterrb.cpp to check for wildcard when at a leaf.
- Adds additional verbose logging to genrb.
- Fixes filtration to add deps to dep_targets instead of dep_files.
- Separates dep_files to common_dep_files and specific_dep_files.
This commit is contained in:
Shane Carr 2019-02-26 12:52:40 -08:00 committed by Shane F. Carr
parent ae13af0a8e
commit 60f4e1ba83
10 changed files with 63 additions and 37 deletions

View file

@ -282,8 +282,7 @@ class ResourceFilterInfo(object):
if request.tool != IcuTool("genrb"):
continue
self._set_files(request.input_files)
# Add dependencies directly to dep_files
request.dep_files += self.filter_files
request.dep_targets += [self.filter_files]
arg_str = "--filterDir {TMP_DIR}/%s" % self.filter_tmp_dir
request.args = "%s %s" % (arg_str, request.args)

View file

@ -26,7 +26,7 @@
"type": "array",
"items": {
"type": "string",
"pattern": "^[+-]/(\\w+(/\\w+)*)?$"
"pattern": "^[+-]/((\\w+|\\*)(/\\w+|/\\*)*)?$"
}
}
},

View file

@ -212,18 +212,18 @@ def get_gnumake_rules_helper(request, common_vars, **kwargs):
rules = []
dep_literals = []
# To keep from repeating the same dep files many times, make a variable.
if len(request.dep_files) > 0:
if len(request.common_dep_files) > 0:
dep_var_name = "%s_DEPS" % request.name.upper()
dep_literals = ["$(%s)" % dep_var_name]
rules += [
MakeFilesVar(
name = dep_var_name,
files = request.dep_files
files = request.common_dep_files
)
]
# Add a rule for each individual file.
for loop_vars in utils.repeated_execution_request_looper(request):
(_, input_file, output_file) = loop_vars
(_, specific_dep_files, input_file, output_file) = loop_vars
name_suffix = input_file[input_file.filename.rfind("/")+1:input_file.filename.rfind(".")]
cmd = utils.format_repeated_request_command(
request,
@ -235,7 +235,7 @@ def get_gnumake_rules_helper(request, common_vars, **kwargs):
MakeRule(
name = "%s_%s" % (request.name, name_suffix),
dep_literals = dep_literals,
dep_files = [input_file],
dep_files = specific_dep_files + [input_file],
output_file = output_file,
cmds = [cmd]
)

View file

@ -56,11 +56,21 @@ class AbstractRequest(object):
class AbstractExecutionRequest(AbstractRequest):
def __init__(self, **kwargs):
# Names of targets (requests) or files that this request depends on;
# targets are of type DepTarget
# Names of targets (requests) or files that this request depends on.
# The entries of dep_targets may be any of the following types:
#
# 1. DepTarget, for the output of an execution request.
# 2. InFile, TmpFile, etc., for a specific file.
# 3. A list of InFile, TmpFile, etc., where the list is the same
# length as self.input_files and self.output_files.
#
# In cases 1 and 2, the dependency is added to all rules that the
# request generates. In case 3, the dependency is added only to the
# rule that generates the output file at the same array index.
self.dep_targets = []
self.dep_files = []
# Computed during self.flatten(); don't edit directly.
self.common_dep_files = []
# Primary input files
self.input_files = []
@ -104,12 +114,24 @@ class AbstractExecutionRequest(AbstractRequest):
if not self.dep_targets:
return
for dep_target in self.dep_targets:
if isinstance(dep_target, InFile):
self.dep_files.append(dep_target)
if isinstance(dep_target, list):
if hasattr(self, "specific_dep_files"):
assert len(dep_target) == len(self.specific_dep_files)
for file, out_list in zip(dep_target, self.specific_dep_files):
assert hasattr(file, "filename")
out_list.append(file)
else:
self.common_dep_files += dep_target
continue
if not isinstance(dep_target, DepTarget):
# Copy file entries directly to dep_files.
assert hasattr(dep_target, "filename")
self.common_dep_files.append(dep_target)
continue
# For DepTarget entries, search for the target.
for request in all_requests:
if request.name == dep_target.name:
self.dep_files += request.all_output_files()
self.common_dep_files += request.all_output_files()
break
else:
print("Warning: Unable to find target %s, a dependency of %s" % (
@ -118,7 +140,7 @@ class AbstractExecutionRequest(AbstractRequest):
), file=sys.stderr)
def all_input_files(self):
return self.dep_files + self.input_files
return self.common_dep_files + self.input_files
def all_output_files(self):
return self.output_files
@ -136,6 +158,9 @@ class RepeatedExecutionRequest(AbstractExecutionRequest):
# iteration; all values must be lists equal in length to input_files
self.repeat_with = {}
# Lists for dep files that are specific to individual resource bundle files
self.specific_dep_files = [[] for _ in range(len(kwargs["input_files"]))]
super(RepeatedExecutionRequest, self).__init__(**kwargs)
def _del_at(self, i):
@ -145,6 +170,12 @@ class RepeatedExecutionRequest(AbstractExecutionRequest):
if isinstance(v, list):
del v[i]
def all_input_files(self):
files = super(RepeatedExecutionRequest, self).all_input_files()
for specific_file_list in self.specific_dep_files:
files += specific_file_list
return files
class RepeatedOrSingleExecutionRequest(AbstractExecutionRequest):
def __init__(self, **kwargs):

View file

@ -43,7 +43,7 @@ def repeated_execution_request_looper(request):
if not ld:
# No special options given in repeat_with
ld = [{} for _ in range(len(request.input_files))]
return zip(ld, request.input_files, request.output_files)
return zip(ld, request.specific_dep_files, request.input_files, request.output_files)
def format_single_request_command(request, cmd_template, common_vars):
@ -57,7 +57,7 @@ def format_single_request_command(request, cmd_template, common_vars):
def format_repeated_request_command(request, cmd_template, loop_vars, common_vars):
(iter_vars, input_file, output_file) = loop_vars
(iter_vars, _, input_file, output_file) = loop_vars
return cmd_template.format(
ARGS = request.args.format(
INPUT_FILE = input_file.filename,

View file

@ -58,7 +58,6 @@ def generate_rb(config, glob, common_vars):
RepeatedExecutionRequest(
name = "testrb",
category = "tests",
dep_files = [],
input_files = [InFile("%s.txt" % bn) for bn in basenames],
output_files = [OutFile("%s.res" % bn) for bn in basenames],
tool = IcuTool("genrb"),
@ -70,7 +69,6 @@ def generate_rb(config, glob, common_vars):
SingleExecutionRequest(
name = "encoded",
category = "tests",
dep_files = [],
input_files = [InFile("encoded.utf16be")],
output_files = [OutFile("encoded.res")],
tool = IcuTool("genrb"),
@ -80,7 +78,6 @@ def generate_rb(config, glob, common_vars):
SingleExecutionRequest(
name = "zoneinfo64",
category = "tests",
dep_files = [],
input_files = [InFile("zoneinfo64.txt")],
output_files = [TmpFile("zoneinfo64.res")],
tool = IcuTool("genrb"),
@ -90,7 +87,6 @@ def generate_rb(config, glob, common_vars):
SingleExecutionRequest(
name = "filtertest",
category = "tests",
dep_files = [],
input_files = [InFile("filtertest.txt")],
output_files = [OutFile("filtertest.res")],
tool = IcuTool("genrb"),
@ -106,7 +102,6 @@ def generate_sprep(config, glob, common_vars):
SingleExecutionRequest(
name = "nfscsi",
category = "tests",
dep_files = [],
input_files = [InFile("nfs4_cs_prep_ci.txt")],
output_files = [OutFile("nfscsi.spp")],
tool = IcuTool("gensprep"),
@ -116,7 +111,6 @@ def generate_sprep(config, glob, common_vars):
SingleExecutionRequest(
name = "nfscss",
category = "tests",
dep_files = [],
input_files = [InFile("nfs4_cs_prep_cs.txt")],
output_files = [OutFile("nfscss.spp")],
tool = IcuTool("gensprep"),
@ -126,7 +120,6 @@ def generate_sprep(config, glob, common_vars):
SingleExecutionRequest(
name = "nfscis",
category = "tests",
dep_files = [],
input_files = [InFile("nfs4_cis_prep.txt")],
output_files = [OutFile("nfscis.spp")],
tool = IcuTool("gensprep"),
@ -136,7 +129,6 @@ def generate_sprep(config, glob, common_vars):
SingleExecutionRequest(
name = "nfsmxs",
category = "tests",
dep_files = [],
input_files = [InFile("nfs4_mixed_prep_s.txt")],
output_files = [OutFile("nfsmxs.spp")],
tool = IcuTool("gensprep"),
@ -146,7 +138,6 @@ def generate_sprep(config, glob, common_vars):
SingleExecutionRequest(
name = "nfsmxp",
category = "tests",
dep_files = [],
input_files = [InFile("nfs4_mixed_prep_p.txt")],
output_files = [OutFile("nfsmxp.spp")],
tool = IcuTool("gensprep"),
@ -171,7 +162,6 @@ def generate_conv(config, glob, common_vars):
RepeatedExecutionRequest(
name = "test_conv",
category = "tests",
dep_files = [],
input_files = [InFile("%s.ucm" % bn) for bn in basenames],
output_files = [OutFile("%s.cnv" % bn) for bn in basenames],
tool = IcuTool("makeconv"),
@ -207,7 +197,6 @@ def generate_other(config, glob, common_vars):
SingleExecutionRequest(
name = "testnorm",
category = "tests",
dep_files = [],
input_files = [InFile("testnorm.txt")],
output_files = [OutFile("testnorm.nrm")],
tool = IcuTool("gennorm2"),
@ -217,7 +206,6 @@ def generate_other(config, glob, common_vars):
SingleExecutionRequest(
name = "test_icu",
category = "tests",
dep_files = [],
input_files = [],
output_files = [OutFile("test.icu")],
tool = IcuTool("gentest"),
@ -227,7 +215,6 @@ def generate_other(config, glob, common_vars):
SingleExecutionRequest(
name = "testtable32_txt",
category = "tests",
dep_files = [],
input_files = [],
output_files = [TmpFile("testtable32.txt")],
tool = IcuTool("gentest"),
@ -237,7 +224,6 @@ def generate_other(config, glob, common_vars):
SingleExecutionRequest(
name = "testtable32_res",
category = "tests",
dep_files = [],
input_files = [TmpFile("testtable32.txt")],
output_files = [OutFile("testtable32.res")],
tool = IcuTool("genrb"),

View file

@ -86,9 +86,6 @@ void SimpleRuleBasedPathFilter::addRule(const ResKeyPath& path, bool inclusionRu
return;
}
fRoot.applyRule(path, path.pieces().begin(), inclusionRule, status);
// DEBUG TIP: Enable the following line to view the inclusion tree:
//print(std::cout);
}
PathFilter::EInclusion SimpleRuleBasedPathFilter::match(const ResKeyPath& path) const {
@ -125,7 +122,7 @@ PathFilter::EInclusion SimpleRuleBasedPathFilter::match(const ResKeyPath& path)
}
// Leaf case 2: input path exactly matches a filter leaf
if (node->fChildren.empty()) {
if (node->isLeaf()) {
isLeaf = true;
}
@ -151,6 +148,10 @@ SimpleRuleBasedPathFilter::Tree::Tree(const Tree& other)
}
}
bool SimpleRuleBasedPathFilter::Tree::isLeaf() const {
return fChildren.empty() && !fWildcard;
}
void SimpleRuleBasedPathFilter::Tree::applyRule(
const ResKeyPath& path,
std::list<std::string>::const_iterator it,
@ -159,7 +160,7 @@ void SimpleRuleBasedPathFilter::Tree::applyRule(
// Base Case
if (it == path.pieces().end()) {
if (isVerbose() && (fIncluded != PARTIAL || !fChildren.empty())) {
if (isVerbose() && (fIncluded != PARTIAL || !isLeaf())) {
std::cout << "genrb info: rule on path " << path
<< " overrides previous rules" << std::endl;
}
@ -223,7 +224,7 @@ void SimpleRuleBasedPathFilter::Tree::print(std::ostream& out, int32_t indent) c
void SimpleRuleBasedPathFilter::print(std::ostream& out) const {
out << "SimpleRuleBasedPathFilter {" << std::endl;
fRoot.print(out, 1);
out << "}";
out << "}" << std::endl;
}
std::ostream& operator<<(std::ostream& out, const SimpleRuleBasedPathFilter& value) {

View file

@ -164,6 +164,8 @@ private:
bool inclusionRule,
UErrorCode& status);
bool isLeaf() const;
void print(std::ostream& out, int32_t indent) const;
};

View file

@ -691,6 +691,10 @@ processFile(const char *filename, const char *cp,
}
}
if (isVerbose()) {
filter.print(std::cout);
}
// Apply the filter to the data
ResKeyPath path;
data->fRoot->applyFilter(filter, path, data.getAlias());

View file

@ -1755,11 +1755,14 @@ void TableResource::applyFilter(
if (inclusion == PathFilter::EInclusion::INCLUDE) {
// Include whole subtree
// no-op
if (isVerbose()) {
std::cout << "genrb subtree: " << bundle->fLocale << ": INCLUDE: " << path << std::endl;
}
} else if (inclusion == PathFilter::EInclusion::EXCLUDE) {
// Reject the whole subtree
// Remove it from the linked list
if (isVerbose()) {
std::cout << "genrb removing subtree: " << bundle->fLocale << ": " << path << std::endl;
std::cout << "genrb subtree: " << bundle->fLocale << ": DELETE: " << path << std::endl;
}
if (prev == nullptr) {
fFirst = curr->fNext;