Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(455)

Side by Side Diff: tools/gn/target.cc

Issue 1126193005: Check for inputs not generated by deps (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@data
Patch Set: comment Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2013 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "tools/gn/target.h" 5 #include "tools/gn/target.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/strings/string_util.h" 8 #include "base/strings/string_util.h"
9 #include "base/strings/stringprintf.h" 9 #include "base/strings/stringprintf.h"
10 #include "tools/gn/config_values_extractors.h" 10 #include "tools/gn/config_values_extractors.h"
(...skipping 42 matching lines...) Expand 10 before | Expand all | Expand 10 after
53 from->label().GetUserVisibleName(false) + 53 from->label().GetUserVisibleName(false) +
54 "\n" 54 "\n"
55 "which is a complete static library can't depend on\n" + 55 "which is a complete static library can't depend on\n" +
56 to->label().GetUserVisibleName(false) + 56 to->label().GetUserVisibleName(false) +
57 "\n" 57 "\n"
58 "which is a static library.\n" 58 "which is a static library.\n"
59 "\n" 59 "\n"
60 "Use source sets for intermediate targets instead."); 60 "Use source sets for intermediate targets instead.");
61 } 61 }
62 62
63 // Set check_private_deps to true for the first invocation since a target
64 // can see all of its dependencies. For recursive invocations this will be set
65 // to false to follow only public dependency paths.
66 //
67 // Pass a pointer to an empty set for the first invocation. This will be used
68 // to avoid duplicate checking.
69 bool EnsureFileIsGeneratedByDependency(const Target* target,
70 const OutputFile& file,
71 bool check_private_deps,
72 std::set<const Target*>* seen_targets) {
73 if (seen_targets->find(target) != seen_targets->end())
74 return false; // Already checked this one and it's not found.
75 seen_targets->insert(target);
76
77 // Assume that we have relatively few generated inputs so brute-force
78 // searching here is OK. If this becomes a bottleneck, consider storing
79 // computed_outputs as a hash set.
80 for (const OutputFile& cur : target->computed_outputs()) {
scottmg 2015/05/14 23:04:04 (this could be std::find, but whatever you prefer)
brettw 2015/05/14 23:21:03 In this case the find code is silly looking becaus
81 if (file == cur)
82 return true;
83 }
84
85 // Check all public dependencies (don't do data ones since those are
86 // runtime-only).
87 for (const auto& pair : target->public_deps()) {
88 if (EnsureFileIsGeneratedByDependency(pair.ptr, file, false,
89 seen_targets))
90 return true; // Found a path.
91 }
92
93 // Only check private deps if requested.
94 if (check_private_deps) {
95 for (const auto& pair : target->private_deps()) {
96 if (EnsureFileIsGeneratedByDependency(pair.ptr, file, false,
97 seen_targets))
98 return true; // Found a path.
99 }
100 }
101 return false;
102 }
103
63 } // namespace 104 } // namespace
64 105
65 Target::Target(const Settings* settings, const Label& label) 106 Target::Target(const Settings* settings, const Label& label)
66 : Item(settings, label), 107 : Item(settings, label),
67 output_type_(UNKNOWN), 108 output_type_(UNKNOWN),
68 all_headers_public_(true), 109 all_headers_public_(true),
69 check_includes_(true), 110 check_includes_(true),
70 complete_static_lib_(false), 111 complete_static_lib_(false),
71 testonly_(false), 112 testonly_(false),
72 toolchain_(nullptr) { 113 toolchain_(nullptr) {
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
125 all_lib_dirs_.append(cur.lib_dirs().begin(), cur.lib_dirs().end()); 166 all_lib_dirs_.append(cur.lib_dirs().begin(), cur.lib_dirs().end());
126 all_libs_.append(cur.libs().begin(), cur.libs().end()); 167 all_libs_.append(cur.libs().begin(), cur.libs().end());
127 } 168 }
128 169
129 PullDependentTargets(); 170 PullDependentTargets();
130 PullForwardedDependentConfigs(); 171 PullForwardedDependentConfigs();
131 PullRecursiveHardDeps(); 172 PullRecursiveHardDeps();
132 173
133 FillOutputFiles(); 174 FillOutputFiles();
134 175
135 if (!CheckVisibility(err)) 176 if (settings()->build_settings()->check_for_bad_items()) {
136 return false; 177 if (!CheckVisibility(err))
137 if (!CheckTestonly(err)) 178 return false;
138 return false; 179 if (!CheckTestonly(err))
139 if (!CheckNoNestedStaticLibs(err)) 180 return false;
140 return false; 181 if (!CheckNoNestedStaticLibs(err))
182 return false;
183 if (!CheckSourcesGenerated(err))
184 return false;
185 }
141 186
142 return true; 187 return true;
143 } 188 }
144 189
145 bool Target::IsLinkable() const { 190 bool Target::IsLinkable() const {
146 return output_type_ == STATIC_LIBRARY || output_type_ == SHARED_LIBRARY; 191 return output_type_ == STATIC_LIBRARY || output_type_ == SHARED_LIBRARY;
147 } 192 }
148 193
149 bool Target::IsFinal() const { 194 bool Target::IsFinal() const {
150 return output_type_ == EXECUTABLE || output_type_ == SHARED_LIBRARY || 195 return output_type_ == EXECUTABLE || output_type_ == SHARED_LIBRARY ||
(...skipping 146 matching lines...) Expand 10 before | Expand all | Expand 10 after
297 // when Android uses a better STL. 342 // when Android uses a better STL.
298 for (std::set<const Target*>::const_iterator cur = 343 for (std::set<const Target*>::const_iterator cur =
299 pair.ptr->recursive_hard_deps().begin(); 344 pair.ptr->recursive_hard_deps().begin();
300 cur != pair.ptr->recursive_hard_deps().end(); ++cur) 345 cur != pair.ptr->recursive_hard_deps().end(); ++cur)
301 recursive_hard_deps_.insert(*cur); 346 recursive_hard_deps_.insert(*cur);
302 } 347 }
303 } 348 }
304 349
305 void Target::FillOutputFiles() { 350 void Target::FillOutputFiles() {
306 const Tool* tool = toolchain_->GetToolForTargetFinalOutput(this); 351 const Tool* tool = toolchain_->GetToolForTargetFinalOutput(this);
352 bool check_tool_outputs = false;
307 switch (output_type_) { 353 switch (output_type_) {
308 case GROUP: 354 case GROUP:
309 case SOURCE_SET: 355 case SOURCE_SET:
310 case COPY_FILES: 356 case COPY_FILES:
311 case ACTION: 357 case ACTION:
312 case ACTION_FOREACH: { 358 case ACTION_FOREACH: {
313 // These don't get linked to and use stamps which should be the first 359 // These don't get linked to and use stamps which should be the first
314 // entry in the outputs. These stamps are named 360 // entry in the outputs. These stamps are named
315 // "<target_out_dir>/<targetname>.stamp". 361 // "<target_out_dir>/<targetname>.stamp".
316 dependency_output_file_ = GetTargetOutputDirAsOutputFile(this); 362 dependency_output_file_ = GetTargetOutputDirAsOutputFile(this);
317 dependency_output_file_.value().append(GetComputedOutputName(true)); 363 dependency_output_file_.value().append(GetComputedOutputName(true));
318 dependency_output_file_.value().append(".stamp"); 364 dependency_output_file_.value().append(".stamp");
319 break; 365 break;
320 } 366 }
321 case EXECUTABLE: 367 case EXECUTABLE:
322 // Executables don't get linked to, but the first output is used for 368 // Executables don't get linked to, but the first output is used for
323 // dependency management. 369 // dependency management.
324 CHECK_GE(tool->outputs().list().size(), 1u); 370 CHECK_GE(tool->outputs().list().size(), 1u);
371 check_tool_outputs = true;
325 dependency_output_file_ = 372 dependency_output_file_ =
326 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( 373 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
327 this, tool, tool->outputs().list()[0]); 374 this, tool, tool->outputs().list()[0]);
328 break; 375 break;
329 case STATIC_LIBRARY: 376 case STATIC_LIBRARY:
330 // Static libraries both have dependencies and linking going off of the 377 // Static libraries both have dependencies and linking going off of the
331 // first output. 378 // first output.
332 CHECK(tool->outputs().list().size() >= 1); 379 CHECK(tool->outputs().list().size() >= 1);
380 check_tool_outputs = true;
333 link_output_file_ = dependency_output_file_ = 381 link_output_file_ = dependency_output_file_ =
334 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( 382 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
335 this, tool, tool->outputs().list()[0]); 383 this, tool, tool->outputs().list()[0]);
336 break; 384 break;
337 case SHARED_LIBRARY: 385 case SHARED_LIBRARY:
338 CHECK(tool->outputs().list().size() >= 1); 386 CHECK(tool->outputs().list().size() >= 1);
387 check_tool_outputs = true;
339 if (tool->link_output().empty() && tool->depend_output().empty()) { 388 if (tool->link_output().empty() && tool->depend_output().empty()) {
340 // Default behavior, use the first output file for both. 389 // Default behavior, use the first output file for both.
341 link_output_file_ = dependency_output_file_ = 390 link_output_file_ = dependency_output_file_ =
342 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( 391 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
343 this, tool, tool->outputs().list()[0]); 392 this, tool, tool->outputs().list()[0]);
344 } else { 393 } else {
345 // Use the tool-specified ones. 394 // Use the tool-specified ones.
346 if (!tool->link_output().empty()) { 395 if (!tool->link_output().empty()) {
347 link_output_file_ = 396 link_output_file_ =
348 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( 397 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
349 this, tool, tool->link_output()); 398 this, tool, tool->link_output());
350 } 399 }
351 if (!tool->depend_output().empty()) { 400 if (!tool->depend_output().empty()) {
352 dependency_output_file_ = 401 dependency_output_file_ =
353 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile( 402 SubstitutionWriter::ApplyPatternToLinkerAsOutputFile(
354 this, tool, tool->depend_output()); 403 this, tool, tool->depend_output());
355 } 404 }
356 } 405 }
357 break; 406 break;
358 case UNKNOWN: 407 case UNKNOWN:
359 default: 408 default:
360 NOTREACHED(); 409 NOTREACHED();
361 } 410 }
411
412 // Count all outputs from this tool as something generated by this target.
413 if (check_tool_outputs) {
414 SubstitutionWriter::ApplyListToLinkerAsOutputFile(
415 this, tool, tool->outputs(), &computed_outputs_);
416 }
417
418 // Also count anything the target has declared to be an output.
419 std::vector<SourceFile> outputs_as_sources;
420 action_values_.GetOutputsAsSourceFiles(this, &outputs_as_sources);
421 for (const SourceFile& out : outputs_as_sources)
422 computed_outputs_.push_back(OutputFile(settings()->build_settings(), out));
362 } 423 }
363 424
364 bool Target::CheckVisibility(Err* err) const { 425 bool Target::CheckVisibility(Err* err) const {
365 for (const auto& pair : GetDeps(DEPS_ALL)) { 426 for (const auto& pair : GetDeps(DEPS_ALL)) {
366 if (!Visibility::CheckItemVisibility(this, pair.ptr, err)) 427 if (!Visibility::CheckItemVisibility(this, pair.ptr, err))
367 return false; 428 return false;
368 } 429 }
369 return true; 430 return true;
370 } 431 }
371 432
(...skipping 30 matching lines...) Expand all
402 463
403 // Verify no inherited libraries are static libraries. 464 // Verify no inherited libraries are static libraries.
404 for (const auto& lib : inherited_libraries().GetOrdered()) { 465 for (const auto& lib : inherited_libraries().GetOrdered()) {
405 if (lib->output_type() == Target::STATIC_LIBRARY) { 466 if (lib->output_type() == Target::STATIC_LIBRARY) {
406 *err = MakeStaticLibDepsError(this, lib); 467 *err = MakeStaticLibDepsError(this, lib);
407 return false; 468 return false;
408 } 469 }
409 } 470 }
410 return true; 471 return true;
411 } 472 }
473
474 bool Target::CheckSourcesGenerated(Err* err) const {
475 // Checks that any inputs or sources to this target that are in the build
476 // directory are generated by a target that this one transitively depends on
477 // in some way. We already guarantee that all generated files are written
478 // to the build dir.
479 for (const SourceFile& file : sources_) {
480 if (!CheckSourceGenerated(file, err))
481 return false;
482 }
483 for (const SourceFile& file : inputs_) {
484 if (!CheckSourceGenerated(file, err))
485 return false;
486 }
487 return true;
488 }
489
490 bool Target::CheckSourceGenerated(const SourceFile& source, Err* err) const {
491 if (!IsStringInOutputDir(settings()->build_settings()->build_dir(),
492 source.value()))
493 return true; // Not in output dir, this is OK.
494
495 OutputFile out_file(settings()->build_settings(), source);
496 std::set<const Target*> seen_targets;
497 if (!EnsureFileIsGeneratedByDependency(this, out_file, true, &seen_targets)) {
498 *err = Err(defined_from(),
499 "Target has an input not generated by a dependency.",
500 "The source or input file\n " + source.value() + "\n"
501 "on the target\n " + label().GetUserVisibleName(false) + "\n"
502 "is in the build directory but was not specified as an output by\n"
503 "any of this target's dependencies or transitive public dependencies.\n"
504 "\n"
505 "If you have generated inputs, there needs to be a dependency path\n"
506 "between the two targets in addition to just listing the files.\n"
507 "For indirect dependencies, the intermediate ones must be\n"
508 "public_deps. data_deps don't count since they're only runtime\n"
509 "dependencies.\n"
510 "\n"
511 "Start with:\n"
512 " gn refs " +
513 DirectoryWithNoLastSlash(settings()->build_settings()->build_dir()) +
514 " " + source.value() + "\n"
515 "which will print all targets that reference the file EITHER as an\n"
516 "input or an output, and go from there.");
517 return false;
518 }
519 return true;
520 }
OLDNEW
« no previous file with comments | « tools/gn/target.h ('k') | tools/gn/target_unittest.cc » ('j') | tools/gn/target_unittest.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698