New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 632412 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit 26 days ago
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

get_label_info() should respect target toolchain for target_gen_dir

Project Member Reported by sdefresne@chromium.org, Jul 28 2016

Issue description

If we have two toolchains, and use get_label_info() for a target in the other toolchain, it should respect the toolchain if specified.

In particular, on iOS, in https://codereview.chromium.org/2187343003, I want to do the following:

if (toolchain == default_toolchain) {
  _xcent_output = "$target_gen_dir/$_output_name.xcent"
  action(_xcent_target) {
    script = "..."
    sources = [ ... ]
    outputs = [ "$target_gen_dir/$_output_name.xcent" ]
  }
} else {
  _xcent_output = get_label_info(
      "$target_gen_dir/$_output_name.xcent($default_toolchain)",
      target_gen_dir) +
    "/$_output_name.xcent"
}

executable(_target_name) {
  deps = [ "$target_gen_dir/$_output_name.xcent($default_toolchain)" ]
  inputs = [ _xcent_output ]
}

 
I expect get_label_info("$target_gen_dir/$_output_name.xcent($default_toolchain)", "target_gen_dir") to be equal to the same thing as "$target_gen_dir/$_output_name.xcent" when toolchain is default_toolchain, but it always contains the current_toolchain prefix.

Comment 2 by brettw@chromium.org, Jul 29 2016

Owner: brettw@chromium.org
Status: Started (was: Untriaged)

Comment 3 by brettw@chromium.org, Jul 29 2016

Summary: get_label_info() should respect target toolchain for target_gen_dir (was: get_label_info() should respect target toolchain)
It looks like this bug is only with "target_gen_dir".

To get you unblocked, you can do:
print(get_label_info(":foo($default_toolchain)", "root_gen_dir") + "/" +
      rebase_path(".", "//"))

"root_gen_dir" works, and then you append the path to the current directory relative to the source root.

Comment 4 by brettw@chromium.org, Jul 29 2016

(Don't do the print thing, that was me testing it)
Project Member

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

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

commit 8d6e145e3c939895491b83219583dfef48a94fac
Author: brettw <brettw@chromium.org>
Date: Tue Aug 02 21:38:59 2016

Make get_label_info take into account the toolchain for target_gen_dir

Previously get_label_info(<label>, "target_gen_dir") discarded toolchain information for the <label> and used the current toolchain. This makes this query useless for anything but the current toolchain. The other label info categories included this properly.

The actual bugfix is passing label.GetToolchainLabel() for the "target_gen_dir" case in function_get_label_info. This is tested by the new test in function-get_label_info_unittest.cc which was missing a test for this case.

Adding the toolchain label required adding more variants of the directory getters in filesystem_utils which were already passed the point of reason in terms of number of variants taking and returning slightly different but related things.

So this does a refactoring of the directory getter functions there to rationalize them among 4 classes of functions which can return one of two things, and a helper class that generalizes the input with whatever information the caller has. The rest of the changes are updates to the callers for this new system. There are some additional cases added to filesystem_utils_unittest.cc to test more combinations.

BUG= 632412 

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

[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/filesystem_utils.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/filesystem_utils.h
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/filesystem_utils_unittest.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/function_get_label_info.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/function_get_label_info_unittest.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/function_get_path_info.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/ninja_binary_target_writer.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/ninja_create_bundle_target_writer.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/ninja_target_writer.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/ninja_utils.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/scope_per_file_provider.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/substitution_writer.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/target.cc
[modify] https://crrev.com/8d6e145e3c939895491b83219583dfef48a94fac/tools/gn/visual_studio_writer.cc

Status: Fixed (was: Started)

Sign in to add a comment