Add node.js gsutil .sha1 files.

Add node.js gsutil .sha1 files.

This adds the gsutil .sha1 files for node.js binaries which are stored in
Google storage. It also updates the DEPS file to pull these binaries.

BUG=

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

[raymes, 2014-01-07 03:16:28]

Hey guys, PTAL. I'm not entirely sure whether this should go in tools/ or in
third_party/.

Thanks!
Raymes

[cmp, 2014-01-07 18:13:26]

+szager for review

[Ryan Tseng, 2014-01-07 21:18:00]

https://codereview.chromium.org/125593004/diff/70001/DEPS
File DEPS (right):

https://codereview.chromium.org/125593004/diff/70001/DEPS#newcode673
DEPS:673: "name": "nodejs_linux",
You can probably combine this and the one below into one.  Just pass in
src/tools/nodejs/linux/*.sha1
as the pattern, 
src/tools/nodejs/linux
as the target, and add an extra --directory flag, and omit the -s flag.

[raymes, 2014-01-07 22:56:54]

Thanks!

https://codereview.chromium.org/125593004/diff/70001/DEPS
File DEPS (right):

https://codereview.chromium.org/125593004/diff/70001/DEPS#newcode673
DEPS:673: "name": "nodejs_linux",
I'm not 100% clear on what you mean :) Could you please give an example of what
it would look like?

Is it better to just keep them separate as-is for clarity?

[Ryan Tseng, 2014-01-07 23:00:45]

https://codereview.chromium.org/125593004/diff/70001/DEPS
File DEPS (right):

https://codereview.chromium.org/125593004/diff/70001/DEPS#newcode673
DEPS:673: "name": "nodejs_linux",
On 2014/01/07 22:56:54, raymes wrote:
> I'm not 100% clear on what you mean :) Could you please give an example of
what
> it would look like?
> 
> Is it better to just keep them separate as-is for clarity?

So this would be

{
  "name":  "nodejs_linux",
  "pattern": "src/tools/nodejs/linux/*.sha1",
  "action": [ "download_from_google_storage',
                 "--no_resume",
                 "--platform=linux*",
                 "--no_auth",
                 "--bucket", "chromium-nodejs",
                 "--directory", "src/tools/nodejs/linux",]
}

I think It makes sense to put them together since they always get triggered
together.  Also that results in less hooks.

[raymes, 2014-01-07 23:03:53]

On 2014/01/07 23:00:45, Ryan T. wrote:
> https://codereview.chromium.org/125593004/diff/70001/DEPS
> File DEPS (right):
> 
> https://codereview.chromium.org/125593004/diff/70001/DEPS#newcode673
> DEPS:673: "name": "nodejs_linux",
> On 2014/01/07 22:56:54, raymes wrote:
> > I'm not 100% clear on what you mean :) Could you please give an example of
> what
> > it would look like?
> > 
> > Is it better to just keep them separate as-is for clarity?
> 
> So this would be
> 
> {
>   "name":  "nodejs_linux",
>   "pattern": "src/tools/nodejs/linux/*.sha1",
>   "action": [ "download_from_google_storage',
>                  "--no_resume",
>                  "--platform=linux*",
>                  "--no_auth",
>                  "--bucket", "chromium-nodejs",
>                  "--directory", "src/tools/nodejs/linux",]
> }
> 
> I think It makes sense to put them together since they always get triggered
> together.  Also that results in less hooks.

Thanks! I updated the patch.

[Ryan Tseng, 2014-01-08 22:59:55]

The DEPS looks good, one more question

https://codereview.chromium.org/125593004/diff/160001/tools/nodejs/README.chr...
File tools/nodejs/README.chromium (right):

https://codereview.chromium.org/125593004/diff/160001/tools/nodejs/README.chr...
tools/nodejs/README.chromium:5: License File: NOT_SHIPPED
the LICENSE file isn't the license?

[raymes, 2014-01-08 23:02:36]

tHANKS!

https://codereview.chromium.org/125593004/diff/160001/tools/nodejs/README.chr...
File tools/nodejs/README.chromium (right):

https://codereview.chromium.org/125593004/diff/160001/tools/nodejs/README.chr...
tools/nodejs/README.chromium:5: License File: NOT_SHIPPED
On 2014/01/08 22:59:55, Ryan T. wrote:
> the LICENSE file isn't the license?

It is but according to
http://src.chromium.org/viewvc/chrome/trunk/src/third_party/README.chromium.t...
I should use NOT_SHIPPED:

"Use the special value NOT_SHIPPED to indicate that the package is not included
in the shipped product, so its license does not need to be included in
about:credits and no license file is required."

[raymes, 2014-01-08 23:02:42]

Thanks!

[Ryan Tseng, 2014-01-08 23:06:28]

Ah I see, lgtm

FYI I have https://codereview.chromium.org/126893002/ to make the DEPS look less
silly with putting in 3 platforms.  Its not relevant right now, but we'll
probably want to change the hooks after that lands.

[raymes, 2014-01-08 23:09:45]

Cool, thanks!

On Thu, Jan 9, 2014 at 10:06 AM,  <hinoka@google.com> wrote:
> Ah I see, lgtm
>
> FYI I have https://codereview.chromium.org/126893002/ to make the DEPS look
> less
> silly with putting in 3 platforms.  Its not relevant right now, but we'll
> probably want to change the hooks after that lands.
>
> https://codereview.chromium.org/125593004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-01-08 23:17:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/125593004/160001

[commit-bot: I haz the power, 2014-01-09 00:12:43]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2014-01-09 00:17:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/125593004/160001

[commit-bot: I haz the power, 2014-01-09 04:37:48]

Change committed as 243768

[jamesr, 2014-01-10 01:14:49]

I'm getting this in my git status output since this patch landed:

Untracked files:
  (use "git add <file>..." to include in what will be committed)

	tools/nodejs/linux/node
	tools/nodejs/linux/node32


this is useless noise now and pretty harmful if I end up accidentally doing
something like 'git add -A'.  Please fix.

[hinoka, 2014-01-10 01:17:39]

The files need to be added to some .gitignore file.  I'll add step to the docs.

[hinoka, 2014-01-10 01:22:21]

Example here:
https://codereview.chromium.org/111953007/diff/100001/.gitignore

[raymes, 2014-01-10 01:51:40]

I'll put up a patch. Thanks for letting us know James.

On Fri, Jan 10, 2014 at 12:22 PM,  <hinoka@chromium.org> wrote:
> Example here:
> https://codereview.chromium.org/111953007/diff/100001/.gitignore
>
> https://codereview.chromium.org/125593004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jamesr1, 2014-01-10 02:03:29]

Thanks!  Be sure to update the docs for future folks using this technique.


On Thu, Jan 9, 2014 at 5:50 PM, Raymes Khoury <raymes@chromium.org> wrote:

> I'll put up a patch. Thanks for letting us know James.
>
> On Fri, Jan 10, 2014 at 12:22 PM,  <hinoka@chromium.org> wrote:
> > Example here:
> > https://codereview.chromium.org/111953007/diff/100001/.gitignore
> >
> > https://codereview.chromium.org/125593004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[raymes, 2014-01-10 03:44:38]

A revert of this CL has been created in
https://codereview.chromium.org/133033003/ by raymes@chromium.org.

The reason for reverting is: The binaries haven't been verified..

[Nico, 2014-01-10 04:28:23]

Sorry, why do we need to have node.js binaries in a chromium checkout? We
already have a copy of v8.

The CL description doesn't have a BUG= line describing what is this for either.

[raymes, 2014-01-10 04:33:27]

It's needed for running vulcanize. Please refer to the discussion in
https://groups.google.com/a/google.com/forum/#!msg/chrome-infrastructure-team...

Sorry for the lack of description in the commit message.

On Fri, Jan 10, 2014 at 3:28 PM,  <thakis@chromium.org> wrote:
> Sorry, why do we need to have node.js binaries in a chromium checkout? We
> already have a copy of v8.
>
> The CL description doesn't have a BUG= line describing what is this for
> either.
>
> https://codereview.chromium.org/125593004/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2014-01-10 22:52:14]

That's an internal mailing list, many contributors won't be able to read
this. When adding dependencies like this, _please_ have a tracking bug that
explains what a dependency is needed for.

In this case, it looks like you're adding this to run a few hundred lines
of javascript at build time. Adding node just for that seems like overkill
to me.

From
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/I8FANt54ArA:

"""In general, we should try to keep the number of languages we use to a
minimum. We try hard to stick to C++ for app code, JS for web code,
ObjC for Mac code, and Python for scripts and glue."""

In other words, we probably shouldn't use JS for build scripts.

Can you think of other ways of achieving what you want to achieve?


On Thu, Jan 9, 2014 at 8:32 PM, Raymes Khoury <raymes@chromium.org> wrote:

> It's needed for running vulcanize. Please refer to the discussion in
>
>
https://groups.google.com/a/google.com/forum/#!msg/chrome-infrastructure-team...
>
> Sorry for the lack of description in the commit message.
>
> On Fri, Jan 10, 2014 at 3:28 PM,  <thakis@chromium.org> wrote:
> > Sorry, why do we need to have node.js binaries in a chromium checkout? We
> > already have a copy of v8.
> >
> > The CL description doesn't have a BUG= line describing what is this for
> > either.
> >
> > https://codereview.chromium.org/125593004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.