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

Issue 1214933007: Normalize paths in GN. (Closed)

Created:
5 years, 5 months ago by brettw
Modified:
5 years, 5 months ago
Reviewers:
dnicoara
CC:
chromium-reviews, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Normalize paths in GN. Most SourceFile creation goes through SourceDir.Resolve which normalizes "." and "..". But there are a few cases that just go through the SourceFile constructor directly. Explicitly normalize those cases. This updates the PathOutput unit test since it was written to assume SourceFiles were not normalized. This is testing escaping which is orthogonal, so I changed the test accordingly. Some changed to OutputFiles which are already considered processed and not normalized, and I just deleted another backslash case which was redundantly testing the same thing. BUG=505816 Committed: https://crrev.com/8440203c77e972d5d5289f75ead381821e0756c5 Cr-Commit-Position: refs/heads/master@{#336854}

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : Actually "git add" the test file. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -7 lines) Patch
M tools/gn/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M tools/gn/gn.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M tools/gn/path_output_unittest.cc View 2 chunks +6 lines, -7 lines 0 comments Download
M tools/gn/source_file.cc View 1 chunk +2 lines, -0 lines 0 comments Download
A tools/gn/source_file_unittest.cc View 1 2 1 chunk +19 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (5 generated)
brettw
5 years, 5 months ago (2015-06-30 18:18:25 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1214933007/20001
5 years, 5 months ago (2015-06-30 18:20:56 UTC) #4
dnicoara
Thank you, tested with "use_ozone=true" and the issue is fixed. lgtm
5 years, 5 months ago (2015-06-30 18:24:33 UTC) #5
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_compile_dbg_32_ng/builds/68394)
5 years, 5 months ago (2015-06-30 18:32:47 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1214933007/40001
5 years, 5 months ago (2015-06-30 19:05:29 UTC) #10
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 5 months ago (2015-06-30 19:39:29 UTC) #11
commit-bot: I haz the power
5 years, 5 months ago (2015-06-30 19:40:11 UTC) #12
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/8440203c77e972d5d5289f75ead381821e0756c5
Cr-Commit-Position: refs/heads/master@{#336854}

Powered by Google App Engine
This is Rietveld 408576698