Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1337953002:
Make generated bin/install_incremental_* script remember its own CHROMIUM_OUTPUT_DIR (Closed)
|
Created:
5 years, 3 months ago by agrieve Modified:
5 years, 3 months ago CC:
chromium-reviews, jbudorick+watch_chromium.org, klundberg+watch_chromium.org, yfriedman+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake generated bin/install_incremental_* script remember its own CHROMIUM_OUTPUT_DIR
BUG=520082
Committed: https://crrev.com/b45d19e19fc8bcb9c68be81f1be5549b1ee4f790
Cr-Commit-Position: refs/heads/master@{#348620}
Patch Set 1 #Patch Set 2 : use a flag for setting the output directory rather than an environment variable #
Total comments: 2
Patch Set 3 : delete os.environ() line that was already supposed to be deleted. #
Depends on Patchset: Messages
Total messages: 27 (9 generated)
agrieve@chromium.org changed reviewers: + jbudorick@chromium.org
jbudorick@chromium.org changed reviewers: + mikecase@chromium.org
+mikecase Where/how did this come up? We've been working on breaking test_runner.py's dependency on CHROMIUM_OUTPUT_DIR in favor of --output-directory.
On 2015/09/11 15:51:56, jbudorick wrote: > +mikecase > > Where/how did this come up? We've been working on breaking test_runner.py's > dependency on CHROMIUM_OUTPUT_DIR in favor of --output-directory. Noticed that it was using the md5bin from my out/Debug even though the script was in out-gn/Debug. I could just as easily add an --output-directory flag if you think the environment is too subtle.
On 2015/09/11 at 15:53:51, agrieve wrote: > On 2015/09/11 15:51:56, jbudorick wrote: > > +mikecase > > > > Where/how did this come up? We've been working on breaking test_runner.py's > > dependency on CHROMIUM_OUTPUT_DIR in favor of --output-directory. > > Noticed that it was using the md5bin from my out/Debug even though the script was in out-gn/Debug. I could just as easily add an --output-directory flag if you think the environment is too subtle. Yeah, that'd be better. The environment variable causes some nasty issues with build system integration.
On 2015/09/11 15:56:22, jbudorick wrote: > On 2015/09/11 at 15:53:51, agrieve wrote: > > On 2015/09/11 15:51:56, jbudorick wrote: > > > +mikecase > > > > > > Where/how did this come up? We've been working on breaking test_runner.py's > > > dependency on CHROMIUM_OUTPUT_DIR in favor of --output-directory. > > > > Noticed that it was using the md5bin from my out/Debug even though the script > was in out-gn/Debug. I could just as easily add an --output-directory flag if > you think the environment is too subtle. > > Yeah, that'd be better. The environment variable causes some nasty issues with > build system integration. Done
https://codereview.chromium.org/1337953002/diff/20001/build/android/gn/create... File build/android/gn/create_incremental_install_script.py (right): https://codereview.chromium.org/1337953002/diff/20001/build/android/gn/create... build/android/gn/create_incremental_install_script.py:43: os.environ['CHROMIUM_OUTPUT_DIR'] = resolve_path(os.pardir) Does it work without this? If so, take this out. If not, why?
https://codereview.chromium.org/1337953002/diff/20001/build/android/gn/create... File build/android/gn/create_incremental_install_script.py (right): https://codereview.chromium.org/1337953002/diff/20001/build/android/gn/create... build/android/gn/create_incremental_install_script.py:43: os.environ['CHROMIUM_OUTPUT_DIR'] = resolve_path(os.pardir) On 2015/09/11 16:47:53, jbudorick wrote: > Does it work without this? If so, take this out. If not, why? Whoops, yep, that should not be there.
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/1337953002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1337953002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: cast_shell_linux on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
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/1337953002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1337953002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
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/1337953002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1337953002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
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/1337953002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1337953002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/b45d19e19fc8bcb9c68be81f1be5549b1ee4f790 Cr-Commit-Position: refs/heads/master@{#348620}
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/b45d19e19fc8bcb9c68be81f1be5549b1ee4f790 Cr-Commit-Position: refs/heads/master@{#348620} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
