Move the Windows sandbox to sandbox/win

Move the Windows sandbox to sandbox/win

This is a rather large refactor to move the Windows sandbox to the right place.

BUG=
TEST=
NOTRY=true
TBR=sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=147151

[jln (very slow on Chromium), 2012-07-12 07:29:46]

Hey Justin,

would you mind me trying to land this now?

There is a little bit of additional complexity because we have 3 "binary" files:
wow_helper/wow_helper.exe
wow_helper/wow_helper.pdb
sandbox_poc/sandbox.ico

And try bots don't handle binary files. So I've cheated a bit to send the try
job, with an extra small patch to exclude those files from the build.

So this will need to land without going through the commit queue.

[jschuh, 2012-07-12 16:58:45]

Please run the win and win_rel try jobs as a full clobber. Also, please add cpu
in as a reviewer, because we definitely want a second set of eyes on this.

[jln (very slow on Chromium), 2012-07-12 17:27:55]

On 2012/07/12 16:58:45, Justin Schuh wrote:
> Please run the win and win_rel try jobs as a full clobber. Also, please add
cpu
> in as a reviewer, because we definitely want a second set of eyes on this.

All the trybots have already been run at full clobber. I'm adding "win" now.

[jschuh, 2012-07-12 17:50:39]

On 2012/07/12 17:27:55, Julien Tinnes wrote:
> All the trybots have already been run at full clobber. I'm adding "win" now.

Cool, that runs the debug component builds, which could flush out any remaining
issues.

[jln (very slow on Chromium), 2012-07-12 20:26:34]

On 2012/07/12 17:50:39, Justin Schuh wrote:
> On 2012/07/12 17:27:55, Julien Tinnes wrote:
> > All the trybots have already been run at full clobber. I'm adding "win" now.
> 
> Cool, that runs the debug component builds, which could flush out any
remaining
> issues.

Try success for that job!

[jln (very slow on Chromium), 2012-07-12 20:43:15]

Since this CL will deprecate fast, I'm asking OWNER approval ahead of time so
that we can then move swiftly.

jam: for content/ and ui/views
sky: for ash/

[jschuh, 2012-07-12 22:06:33]

Looks all mechanical, so lgtm on two very important conditions:

1) Please verify that this preserves the svn history (maybe with @maruel)
2) Please inform the sheriff and babysit the tree when this lands.

[jln (very slow on Chromium), 2012-07-12 23:00:58]

On 2012/07/12 22:06:33, Justin Schuh wrote:
> Looks all mechanical, so lgtm on two very important conditions:
> 
> 1) Please verify that this preserves the svn history (maybe with @maruel)

Adding maruel. But I don't think it is possible at all. SVN is too dumb for
that. For SVN to understand it, we'd need 1 commit to move, then 1 commit with
the modification. But of course the first commit would break the tree, and have
an impact on any future bisection.

"git blame" etc should still work though for people using that.

Marc-Antoine, is there another way ?

> 2) Please inform the sheriff and babysit the tree when this lands.
Sure!

[M-A Ruel, 2012-07-13 00:42:25]

On 2012/07/12 23:00:58, Julien Tinnes wrote:
> On 2012/07/12 22:06:33, Justin Schuh wrote:
> > Looks all mechanical, so lgtm on two very important conditions:
> > 
> > 1) Please verify that this preserves the svn history (maybe with @maruel)
> 
> Adding maruel. But I don't think it is possible at all. SVN is too dumb for
> that. For SVN to understand it, we'd need 1 commit to move, then 1 commit with
> the modification. But of course the first commit would break the tree, and
have
> an impact on any future bisection.

1. subversion supports that.
2. As I documented ad-nauseum, it is supported by the CQ.
3. Look at any raw diff and you can see the diff is correctly formatted, since
it was generated with git, example:
https://chromiumcodereview.appspot.com/download/issue10689170_1_3034.diff
4. The CQ supports that, as documented.

> "git blame" etc should still work though for people using that.

Yes.

> Marc-Antoine, is there another way ?

It works, you don't need another way.

[jln (very slow on Chromium), 2012-07-13 00:46:28]

On 2012/07/13 00:42:25, Marc-Antoine Ruel wrote:
> 1. subversion supports that.
> 2. As I documented ad-nauseum, it is supported by the CQ.
> 3. Look at any raw diff and you can see the diff is correctly formatted, since
> it was generated with git, example:
> https://chromiumcodereview.appspot.com/download/issue10689170_1_3034.diff
> 4. The CQ supports that, as documented.

Amazing!

Does this mean I should go through the CQ instead of using dcommit to commit via
SVN? Try bots won't work (because we're moving binary files), so I should just
NOTRY=true but go through the commit queue, is that correct?

Thanks!

[M-A Ruel, 2012-07-13 00:50:34]

On 2012/07/13 00:46:28, Julien Tinnes wrote:
> On 2012/07/13 00:42:25, Marc-Antoine Ruel wrote:
> > 1. subversion supports that.
> > 2. As I documented ad-nauseum, it is supported by the CQ.
> > 3. Look at any raw diff and you can see the diff is correctly formatted,
since
> > it was generated with git, example:
> > https://chromiumcodereview.appspot.com/download/issue10689170_1_3034.diff
> > 4. The CQ supports that, as documented.
> 
> Amazing!
> 
> Does this mean I should go through the CQ instead of using dcommit to commit
via
> SVN? Try bots won't work (because we're moving binary files), so I should just
> NOTRY=true but go through the commit queue, is that correct?

Exact.

> Thanks!

Np. I didn't know Justin was anal about svn history. :P

[jam, 2012-07-13 15:39:14]

content lgtm. not an owner in ui/views...  in the future, feel free to TBR
changes like this, since you're just updating include paths

as for svn history, yes we definitely need to preserve that!

[cpu_(ooo_6.6-7.5), 2012-07-13 15:47:38]

lgtm

[commit-bot: I haz the power, 2012-07-13 23:17:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jln@chromium.org/10689170/26066

[commit-bot: I haz the power, 2012-07-13 23:18:43]

Failed to request the patch to try. Please note that binary filesare still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 404: Not Found

[jln (very slow on Chromium), 2012-07-14 01:50:14]

On 2012/07/13 23:18:43, I haz the power (commit-bot) wrote:
> Failed to request the patch to try. Please note that binary filesare still
> unsupported at the moment, this is being worked on.
> 
> Thanks for your patience.
> 
> HTTP Error 404: Not Found

Marc-Antoine: it looks like even without NOTRY=true I can't use the CQ
unfortunately.

[rvargas (doing something else), 2012-07-16 21:52:38]

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
File sandbox/win/sandbox_win.gypi (right):

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:169: '../..',
Do we still need this?

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:175: #FIXME just for trybot
Not cool

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:306: #FIXME - just for trybot
err... search?

[jln (very slow on Chromium), 2012-07-16 22:12:36]

PS: I might try to land this again on Wednesday which is my "low productivity
day".

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
File sandbox/win/sandbox_win.gypi (right):

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:169: '../..',
On 2012/07/16 21:52:38, rvargas wrote:
> Do we still need this?

I don't know, but if we don't, I would rather make that change afterwards, when
it's not mega hairy and can't go through try jobs :)

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:175: #FIXME just for trybot
On 2012/07/16 21:52:38, rvargas wrote:
> Not cool

Yep, very unfortunate, but trybots don't support binary files.

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:306: #FIXME - just for trybot
On 2012/07/16 21:52:38, rvargas wrote:
> err... search?

Ditto.

[rvargas (doing something else), 2012-07-16 22:26:02]

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
File sandbox/win/sandbox_win.gypi (right):

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:175: #FIXME just for trybot
On 2012/07/16 22:12:36, Julien Tinnes wrote:
> On 2012/07/16 21:52:38, rvargas wrote:
> > Not cool
> 
> Yep, very unfortunate, but trybots don't support binary files.

Sure, but it seems like you are already trying to land the patch, and this
should be fixed before landing.

[jln (very slow on Chromium), 2012-07-16 22:46:15]

On 2012/07/16 22:26:02, rvargas wrote:
>
https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
> File sandbox/win/sandbox_win.gypi (right):
> 
>
https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
> sandbox/win/sandbox_win.gypi:175: #FIXME just for trybot
> On 2012/07/16 22:12:36, Julien Tinnes wrote:
> > On 2012/07/16 21:52:38, rvargas wrote:
> > > Not cool
> > 
> > Yep, very unfortunate, but trybots don't support binary files.
> 
> Sure, but it seems like you are already trying to land the patch, and this
> should be fixed before landing.

Ohh, you're right, my recollection of the timing was wrong. Thanks.

[cpu_(ooo_6.6-7.5), 2012-07-17 19:50:58]

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
File sandbox/win/sandbox_win.gypi (right):

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:178: ],
Hmm I didn't see this. Please fix.

[jln (very slow on Chromium), 2012-07-17 23:01:55]

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
File sandbox/win/sandbox_win.gypi (right):

https://chromiumcodereview.appspot.com/10689170/diff/27132/sandbox/win/sandbo...
sandbox/win/sandbox_win.gypi:178: ],
On 2012/07/17 19:50:59, cpu wrote:
> Hmm I didn't see this. Please fix.

Done.

FYI: this was not part of what I was trying to send via SVN, but indeed this
should never have ended up here.
FYI2: I'm using https://chromiumcodereview.appspot.com/10783004/ for the trybot
version

[M-A Ruel, 2012-07-17 23:35:48]

Ah right, there's still a bug related to binary files I'll flesh out. Just
commit manually.

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[M-A Ruel, 2012-07-17 23:35:49]

Ah right, there's still a bug related to binary files I'll flesh out. Just
commit manually.