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

Issue 32063003: Will now copy apprtc to out/ in order to resolve symlinks on Windows. (Closed)

Created:
7 years, 2 months ago by phoglund_chromium
Modified:
7 years, 2 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Will now copy apprtc to out/ in order to resolve symlinks on Windows. Turns out shutil.copytree automatically resolves symlinks, even on Windows. This solves the problem we had with the adapter.js file as described in https://code.google.com/p/webrtc/issues/detail?id=2542. BUG=webrtc:2542 R=kjellander@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=229781

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 1

Patch Set 4 : #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -0 lines) Patch
M DEPS View 1 2 1 chunk +7 lines, -0 lines 0 comments Download
A copy_apprtc.py View 1 2 3 1 chunk +19 lines, -0 lines 3 comments Download

Messages

Total messages: 9 (0 generated)
phoglund_chromium
7 years, 2 months ago (2013-10-21 09:37:34 UTC) #1
kjellander_chromium
lgtm I assume you'll discard http://review.webrtc.org/2579004/ then?
7 years, 2 months ago (2013-10-21 09:51:17 UTC) #2
phoglund_chromium
Please re-review: turns out copytree fails if the dir is already there. Also not copying ...
7 years, 2 months ago (2013-10-21 11:43:53 UTC) #3
kjellander_chromium
lgtm with a documentation suggestion. https://codereview.chromium.org/32063003/diff/40002/copy_apprtc.py File copy_apprtc.py (right): https://codereview.chromium.org/32063003/diff/40002/copy_apprtc.py#newcode6 copy_apprtc.py:6: """Moves Apprtc to the ...
7 years, 2 months ago (2013-10-21 11:46:59 UTC) #4
phoglund_chromium
Committed patchset #4 manually as r229781 (presubmit successful).
7 years, 2 months ago (2013-10-21 11:51:33 UTC) #5
Ami GONE FROM CHROMIUM
https://codereview.chromium.org/32063003/diff/20001/copy_apprtc.py File copy_apprtc.py (right): https://codereview.chromium.org/32063003/diff/20001/copy_apprtc.py#newcode6 copy_apprtc.py:6: """Moves Apprtc to the out/ directory, where the browser ...
7 years, 2 months ago (2013-10-21 15:01:57 UTC) #6
phoglund_chromium
https://codereview.chromium.org/32063003/diff/20001/copy_apprtc.py File copy_apprtc.py (right): https://codereview.chromium.org/32063003/diff/20001/copy_apprtc.py#newcode17 copy_apprtc.py:17: shutil.rmtree('src/out/apprtc', ignore_errors=True); The problem is I don't have a ...
7 years, 2 months ago (2013-10-21 15:12:36 UTC) #7
Ami GONE FROM CHROMIUM
On Mon, Oct 21, 2013 at 8:12 AM, <phoglund@chromium.org> wrote: > The problem is I ...
7 years, 2 months ago (2013-10-21 15:31:35 UTC) #8
phoglund_chromium
7 years, 2 months ago (2013-10-22 11:24:09 UTC) #9
Message was sent while issue was closed.
> The standard way to inject non-chromium gyp code into chromium's gyp forest
> is a DEPS solution that pulls down a foo/supplement.gypi file into
> chromium's src/ dir.  gyp globs */supplement.gypi and includes anything it
> finds (
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/gyp_chromium...
> ).

Oh, so that's what those are for. Cool, I'll see if I can make a solution that
will do just that.

> 
> I realized after sending my previous comment that you'd already landed
> this.  So take this thread of comments as just an FYI in case you want to
> clean up.
> 

Right, thanks a lot!

Powered by Google App Engine
This is Rietveld 408576698