|
|
Chromium Code Reviews|
Created:
4 years, 6 months ago by agrieve Modified:
4 years, 6 months ago Reviewers:
Dirk Pranke CC:
chromium-reviews, michaelbai Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix jmake root_build_dir vs root_out_dir
BUG=616819
Committed: https://crrev.com/5513a1a7d3a5658e192405e432f67df63acf4b0d
Cr-Commit-Position: refs/heads/master@{#398778}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 17 (7 generated)
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2037113002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2037113002/1
Description was changed from ========== Fix jmake root_build_dir vs root_out_dir BUG=616819 ========== to ========== Fix jmake root_build_dir vs root_out_dir BUG=616819 ==========
agrieve@chromium.org changed reviewers: + dpranke@chromium.org
On 2016/06/03 15:48:42, commit-bot: I haz the power wrote: > Dry run: CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/2037113002/1 > View timeline at > https://chromium-cq-status.appspot.com/patch-timeline/2037113002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2037113002/diff/1/build/config/android/intern... File build/config/android/internal_rules.gni (right): https://codereview.chromium.org/2037113002/diff/1/build/config/android/intern... build/config/android/internal_rules.gni:1583: deps += [ "//third_party/jmake($default_toolchain)" ] Are you sure this shouldn't be ($host_toolchain) ? Why would we run jmake on the target?
https://codereview.chromium.org/2037113002/diff/1/build/config/android/intern... File build/config/android/internal_rules.gni (right): https://codereview.chromium.org/2037113002/diff/1/build/config/android/intern... build/config/android/internal_rules.gni:1583: deps += [ "//third_party/jmake($default_toolchain)" ] On 2016/06/03 18:19:34, Dirk Pranke wrote: > Are you sure this shouldn't be ($host_toolchain) ? Why would we run jmake on the > target? Toolchains & targets are a funny thing. Arguably we'd want to have a toolchain just for java targets. E.g. with monochrome, we'll be rebuilding all the java for arm64 and arm32, even though it's identical. For this change, I figured using the version of jmake in the outermost build directory made the most sense.
On 2016/06/03 18:49:58, agrieve wrote: > https://codereview.chromium.org/2037113002/diff/1/build/config/android/intern... > File build/config/android/internal_rules.gni (right): > > https://codereview.chromium.org/2037113002/diff/1/build/config/android/intern... > build/config/android/internal_rules.gni:1583: deps += [ > "//third_party/jmake($default_toolchain)" ] > On 2016/06/03 18:19:34, Dirk Pranke wrote: > > Are you sure this shouldn't be ($host_toolchain) ? Why would we run jmake on > the > > target? > > Toolchains & targets are a funny thing. Arguably we'd want to have a toolchain > just for java targets. E.g. with monochrome, we'll be rebuilding all the java > for arm64 and arm32, even though it's identical. > > For this change, I figured using the version of jmake in the outermost build > directory made the most sense. bump
lgtm
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2037113002/1
Message was sent while issue was closed.
Description was changed from ========== Fix jmake root_build_dir vs root_out_dir BUG=616819 ========== to ========== Fix jmake root_build_dir vs root_out_dir BUG=616819 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Fix jmake root_build_dir vs root_out_dir BUG=616819 ========== to ========== Fix jmake root_build_dir vs root_out_dir BUG=616819 Committed: https://crrev.com/5513a1a7d3a5658e192405e432f67df63acf4b0d Cr-Commit-Position: refs/heads/master@{#398778} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/5513a1a7d3a5658e192405e432f67df63acf4b0d Cr-Commit-Position: refs/heads/master@{#398778} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
