Skip to content

Commit ca44933

Browse files
committed
search: handle arbitrarily deep self requires references
This requires that all requirements of the current node be processed before starting on any of its children, otherwise we may not select all of the correct components from those children. I still don't think the algorithm is fundamentally correct, since we could still end up visiting the same child more than once, and could fail to get all of the requirements we need for each child before visiting it. However, this is an improvement over the status quo, and does fix the linked issue. Fixes: #101
1 parent 1c10040 commit ca44933

3 files changed

Lines changed: 62 additions & 15 deletions

File tree

src/cps/search.cpp

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -378,13 +378,50 @@ namespace cps::search {
378378
/// @param components the components required from this node
379379
void set_components(std::shared_ptr<Node> node, const std::vector<std::string> & components,
380380
bool default_components) {
381-
// Set the components that this package's depndees want
381+
// If this package needs the default_components, then add to the list of used components
382382
if (default_components && node->data.package.default_components) {
383383
const std::vector<std::string> & defs = node->data.package.default_components.value();
384384
node->data.components.insert(node->data.components.end(), defs.begin(), defs.end());
385385
}
386+
// Then add all of the explicitly listed components
386387
node->data.components.insert(node->data.components.end(), components.begin(), components.end());
387388

389+
// Handle self requirements first
390+
// This takes the form `"requires": [":a", ":b"]`
391+
// These must be handled before child dependencies, as they may alter the requirements placed on the
392+
// children..
393+
std::vector<std::string> self_requires = node->data.components;
394+
std::unordered_set<std::string> processed;
395+
bool self_defaults = default_components;
396+
while (!self_requires.empty()) {
397+
const std::string c_name = self_requires.back();
398+
self_requires.pop_back();
399+
if (processed.find(c_name) != processed.end()) {
400+
continue;
401+
}
402+
processed.emplace(c_name);
403+
404+
const loader::Component & component = node->data.package.components.at(c_name);
405+
auto && required = process_requires(component.require);
406+
if (auto && self = required.find(""); self != required.end()) {
407+
// Don't insert these twice
408+
if (!self_defaults && self->second.defaults && node->data.package.default_components) {
409+
self_defaults = true;
410+
const std::vector<std::string> & defs = node->data.package.default_components.value();
411+
node->data.components.insert(node->data.components.end(), defs.begin(), defs.end());
412+
}
413+
for (auto && comp : self->second.components) {
414+
if (processed.find(comp) != processed.end()) {
415+
continue;
416+
}
417+
self_requires.emplace_back(comp);
418+
node->data.components.emplace_back(comp);
419+
}
420+
}
421+
}
422+
423+
// Walk the list of components for this component, adding component
424+
// requirements recursively for external requirements.
388425
for (const std::string & c_name : node->data.components) {
389426
// It's possible that the Package::Requires section listed
390427
// dependencies we don't actually need. If we don't need them we
@@ -401,16 +438,6 @@ namespace cps::search {
401438
}
402439
}
403440
node->depends = trimmed;
404-
405-
if (auto && self = required.find(""); self != required.end()) {
406-
// Don't insert these twice
407-
if (!default_components && self->second.defaults && node->data.package.default_components) {
408-
const std::vector<std::string> & defs = node->data.package.default_components.value();
409-
node->data.components.insert(node->data.components.end(), defs.begin(), defs.end());
410-
}
411-
node->data.components.insert(node->data.components.end(), self->second.components.begin(),
412-
self->second.components.end());
413-
}
414441
}
415442
}
416443

tests/cases/cps-config.toml

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,13 @@ args = ["flags", "--component", "star_values_override", "--cflags", "--print-err
123123
expected = "-fvectorize -I/usr/local/include -I/opt/include -DBAR=2 -DFOO=1 -DOTHER"
124124

125125
[[case]]
126-
name = "Sets compat_version"
127-
cps = "needs-compat-version"
128-
args = ["flags", "--libs", "--print-errors"]
129-
expected = "-l/usr/lib/libfoo.a"
126+
name = "requires component from self"
127+
cps = "full"
128+
args = ["flags", "--component", "requires-self-helper", "--cflags", "--print-errors"]
129+
expected = "-fvectorize -I/usr/local/include -I/opt/include -DBAR=2 -DFOO=1 -DOTHER"
130+
131+
[[case]]
132+
name = "requires component from self nested"
133+
cps = "full"
134+
args = ["flags", "--component", "requires-self", "--cflags", "--print-errors"]
135+
expected = "-fvectorize -I/usr/local/include -I/opt/include -DBAR=2 -DFOO=1 -DOTHER"

tests/cps-files/lib/cps/full.cps

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,20 @@
112112
},
113113
"location": "/something/lib/libfoo.so.1.2.0",
114114
"link_location": "/something/lib/libfoo.so"
115+
},
116+
"requires-self-helper": {
117+
"type": "interface",
118+
"requires": [
119+
":star_values"
120+
]
121+
},
122+
"requires-self": {
123+
"type": "dylib",
124+
"location": "/something/lib/libbar.so.1.8.1",
125+
"link_location": "/something/lib/libbar.so",
126+
"requires": [
127+
":requires-self-helper"
128+
]
115129
}
116130
},
117131
"default_components": [

0 commit comments

Comments
 (0)