third_party: Add WDS library

third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the 'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within sandbox).

NOPRESUBMIT=true
BUG=242107
Committed: https://crrev.com/6d56dcc319b7c41dacc1fb0977b8980c301f071b
Cr-Commit-Position: refs/heads/master@{#375159}

[Mikhail, 2015-12-04 12:51:03]

Patchset #1 (id:1) has been deleted

[Mikhail, 2015-12-04 13:54:43]

Patchset #1 (id:20001) has been deleted

[Mikhail, 2015-12-04 14:23:23]

Patchset #1 (id:40001) has been deleted

[Mikhail, 2015-12-04 14:23:29]

Patchset #1 (id:60001) has been deleted

[Daniel Berlin, 2015-12-04 17:21:40]

dannyb@google.com changed reviewers:
+ dannyb@google.com

[Daniel Berlin, 2015-12-04 17:21:41]

The license here is listed as LGPL (it should be LGPL 2.0, according to the
license file).

However, *none* of these files have license headers.

So who says the license is LGPL, exactly?

[Lei Zhang, 2015-12-04 18:17:19]

On 2015/12/04 17:21:41, Daniel Berlin wrote:
> The license here is listed as LGPL (it should be LGPL 2.0, according to the
> license file).
> 
> However, *none* of these files have license headers.
> 
> So who says the license is LGPL, exactly?

Presumably in the wds.git repo pulled in via the DEPS file? Except I don't see a
wds repo on https://chromium.googlesource.com .

[Lei Zhang, 2015-12-04 18:24:04]

thestig@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2015-12-04 18:24:05]

Once you get signed off for security and licensing, please file an
infrastructure bug to get https://github.com/01org/wds mirrored by
https://chromium.googlesource.com, otherwise the new DEPS file entry won't work.

Please also run tools/checklicenses/checklicenses.py and make sure its happy.

https://codereview.chromium.org/1498473010/diff/80001/third_party/wds/README....
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/80001/third_party/wds/README....
third_party/wds/README.chromium:9: WDS is a set of libraries for developers who
want to build Wi-Fi Display applications on linux.
80 cols

https://codereview.chromium.org/1498473010/diff/80001/third_party/wds/wds.gyp
File third_party/wds/wds.gyp (right):

https://codereview.chromium.org/1498473010/diff/80001/third_party/wds/wds.gyp...
third_party/wds/wds.gyp:1: # Copyright 2014 The Chromium Authors. All rights
reserved.
2015, which is almost over.

[Mikhail, 2015-12-07 09:15:54]

On 2015/12/04 17:21:41, Daniel Berlin wrote:
> The license here is listed as LGPL (it should be LGPL 2.0, according to the
> license file).
> 
> However, *none* of these files have license headers.
> 
> So who says the license is LGPL, exactly?

The whole WDS is licensed with LGPL 2.1 (or later) licence.
These are bison/flex-generated files, the licence header is now added when
generated.

These files are added to the git tree in order to avoid adding a dependency to
bison/flex for Chromium on Linux(CrOS).

[Mikhail, 2015-12-07 09:21:53]

Thanks for having a look, I'll make a ticked for infra to get a mirror for WDS
repo once this CL is approved (as described at
https://www.chromium.org/developers/adding-3rd-party-libraries).

https://codereview.chromium.org/1498473010/diff/80001/third_party/wds/README....
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/80001/third_party/wds/README....
third_party/wds/README.chromium:9: WDS is a set of libraries for developers who
want to build Wi-Fi Display applications on linux.
On 2015/12/04 18:24:05, Lei Zhang wrote:
> 80 cols

Done.

https://codereview.chromium.org/1498473010/diff/80001/third_party/wds/wds.gyp
File third_party/wds/wds.gyp (right):

https://codereview.chromium.org/1498473010/diff/80001/third_party/wds/wds.gyp...
third_party/wds/wds.gyp:1: # Copyright 2014 The Chromium Authors. All rights
reserved.
On 2015/12/04 18:24:05, Lei Zhang wrote:
> 2015, which is almost over.

Done.

[Mikhail, 2016-02-05 14:50:34]

On 2015/12/04 18:17:19, Lei Zhang wrote:
> Presumably in the wds.git repo pulled in via the DEPS file? Except I don't see
a
> wds repo on https://chromium.googlesource.com .

Update: now WDS has got both licence and security approval, the infrastructure
bug to get WDS repo mirrored by https://chromium.googlesource.com is at
https://code.google.com/p/chromium/issues/detail?id=583589

[Lei Zhang, 2016-02-05 23:00:06]

With the code checked out, does it pass checklicenses.py?

https://codereview.chromium.org/1498473010/diff/100001/DEPS
File DEPS (right):

https://codereview.chromium.org/1498473010/diff/100001/DEPS#newcode427
DEPS:427: # Wireless Display Software For Linux.
For ChromeOS?

https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/README...
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/README...
third_party/wds/README.chromium:6: Security Critical: no
This probably should say yes.

https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/wds.gyp
File third_party/wds/wds.gyp (right):

https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/wds.gy...
third_party/wds/wds.gyp:13: 'type': 'static_library',
Can LGPL code be statically linked?

[Mikhail, 2016-02-08 15:12:18]

https://codereview.chromium.org/1498473010/diff/100001/DEPS
File DEPS (right):

https://codereview.chromium.org/1498473010/diff/100001/DEPS#newcode427
DEPS:427: # Wireless Display Software For Linux.
On 2016/02/05 23:00:05, Lei Zhang wrote:
> For ChromeOS?

"Wireless Display Software For Linux" is the name of the component, but I agree
that it's better to point out which platform is using it.

https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/README...
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/README...
third_party/wds/README.chromium:6: Security Critical: no
On 2016/02/05 23:00:05, Lei Zhang wrote:
> This probably should say yes.
The library code will be compiled conditionally (when 'enable_wifi_display' flag
is set) so it won't be in releases by default. But guess it still requires
strict security policy so I'll update this, thanks.

https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/wds.gyp
File third_party/wds/wds.gyp (right):

https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/wds.gy...
third_party/wds/wds.gyp:13: 'type': 'static_library',
On 2016/02/05 23:00:05, Lei Zhang wrote:
> Can LGPL code be statically linked?
I think it is allowed since Chromium code is open "so that a user has the
opportunity to modify the library and relink the application"
http://www.gnu.org/licenses/gpl-faq.html#LGPLStaticVsDynamic. There are few
existing statically linked LGPL-licensed third-parties in Chromium (e.g.
libusb).

[Mikhail, 2016-02-08 15:25:05]

On 2016/02/05 23:00:06, Lei Zhang wrote:
> With the code checked out, does it pass checklicenses.py?
> 

That gives one complaint for bison-generated header:
'third_party/wds/gen/parser.h' has non-whitelisted license 'GPL (v3 or later)
LGPL (v2.1 or later) with Bison parser exception' 
however having of "GPL (v3 or later) with Bison exception" was discussed and
approved during licensing review of this component.

[chromium-reviews, 2016-02-08 16:26:54]

Please do not *not* statically link LGPL code into chromium if you can *at
all* avoid it.
We statically link only a small number of libraries on a small number of
platforms, and don't want it spreading if we can avoid it.

(for example, libusb is only linked on linux)

On Mon, Feb 8, 2016 at 7:12 AM, <mikhail.pozdnyakov@intel.com> wrote:

>
> https://codereview.chromium.org/1498473010/diff/100001/DEPS
> File DEPS (right):
> https://codereview.chromium.org/1498473010/diff/100001/DEPS#newcode427
> DEPS:427
<https://codereview.chromium.org/1498473010/diff/100001/DEPS#newcode427DEPS:427>:
# Wireless Display Software For Linux.
> On 2016/02/05 23:00:05, Lei Zhang wrote:
> > For ChromeOS?
>
> "Wireless Display Software For Linux" is the name of the component, but
> I agree that it's better to point out which platform is using it.
>
https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/README...
> File third_party/wds/README.chromium (right):
>
https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/README...
> third_party/wds/README.chromium:6
<https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/README...>:
Security Critical: no
> On 2016/02/05 23:00:05, Lei Zhang wrote:
> > This probably should say yes.
> The library code will be compiled conditionally (when
> 'enable_wifi_display' flag is set) so it won't be in releases by
> default. But guess it still requires strict security policy so I'll
> update this, thanks.
>
https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/wds.gyp
> File third_party/wds/wds.gyp (right):
>
https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/wds.gy...
> third_party/wds/wds.gyp:13
<https://codereview.chromium.org/1498473010/diff/100001/third_party/wds/wds.gy...>:
'type': 'static_library',
> On 2016/02/05 23:00:05, Lei Zhang wrote:
> > Can LGPL code be statically linked?
> I think it is allowed since Chromium code is open "so that a user has
> the opportunity to modify the library and relink the
application"http://www.gnu.org/licenses/gpl-faq.html#LGPLStaticVsDynamic. There
are
> few existing statically linked LGPL-licensed third-parties in Chromium
> (e.g. libusb).
> https://codereview.chromium.org/1498473010/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Lei Zhang, 2016-02-08 18:58:33]

On 2016/02/08 15:25:05, Mikhail wrote:
> On 2016/02/05 23:00:06, Lei Zhang wrote:
> > With the code checked out, does it pass checklicenses.py?
> > 
> 
> That gives one complaint for bison-generated header:
> 'third_party/wds/gen/parser.h' has non-whitelisted license 'GPL (v3 or later)
> LGPL (v2.1 or later) with Bison parser exception' 
> however having of "GPL (v3 or later) with Bison exception" was discussed and
> approved during licensing review of this component.

Great, but if checklicenses.py is failing, don't you need to fix it as part of
this CL?

[Mikhail, 2016-02-09 15:55:40]

On 2016/02/08 18:58:33, Lei Zhang wrote:
> 
> Great, but if checklicenses.py is failing, don't you need to fix it as part of
> this CL?
Right, I've fixed that in the latest patch.

[Mikhail, 2016-02-09 15:57:48]

On 2016/02/08 16:26:54, chromium-reviews wrote:
> Please do not *not* statically link LGPL code into chromium if you can *at
> all* avoid it.
> We statically link only a small number of libraries on a small number of
> platforms, and don't want it spreading if we can avoid it.
> 
> (for example, libusb is only linked on linux)
> 
Switched to dynamic linkage, thanks.

[Lei Zhang, 2016-02-09 19:07:52]

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/README...
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/README...
third_party/wds/README.chromium:3: Version: 0.1.0
Should their be a version bump? Patch set 3 says it updated the version number.

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/gen/pa...
File third_party/wds/gen/parser.h (right):

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/gen/pa...
third_party/wds/gen/parser.h:68: #include "libwds/rtsp/audiocodecs.h"
If you fix these up to have the full path instead of a relative path here and in
parser.cpp, then you can probably omit third_party/wds/DEPS and there won't be
any confusion as to which libwds directory this is.

https://codereview.chromium.org/1498473010/diff/120001/tools/checklicenses/ch...
File tools/checklicenses/checklicenses.py (right):

https://codereview.chromium.org/1498473010/diff/120001/tools/checklicenses/ch...
tools/checklicenses/checklicenses.py:68: 'GPL (v3 or later) LGPL (v2.1 or later)
with Bison parser exception',
Alphabetical order please. Best to sort with LANG=C.

[Mikhail, 2016-02-10 11:34:37]

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/README...
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/README...
third_party/wds/README.chromium:3: Version: 0.1.0
On 2016/02/09 19:07:52, Lei Zhang wrote:
> Should their be a version bump? Patch set 3 says it updated the version
number.

Right, thanks for pointing out.

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/gen/pa...
File third_party/wds/gen/parser.h (right):

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/gen/pa...
third_party/wds/gen/parser.h:68: #include "libwds/rtsp/audiocodecs.h"
On 2016/02/09 19:07:52, Lei Zhang wrote:
> If you fix these up to have the full path instead of a relative path here and
in
> parser.cpp, then you can probably omit third_party/wds/DEPS and there won't be
> any confusion as to which libwds directory this is.

I've tried this and faced with following drawbacks. 
- changing the include paths in the generated files complicates the process of
updating WDS to a new version
- wds.gyp contains deps (include path) to upper level directories

IMO having a third_party/wds/DEPS is bit simpler, but if you prefer the other
solution I'll update the patch accordingly.

https://codereview.chromium.org/1498473010/diff/120001/tools/checklicenses/ch...
File tools/checklicenses/checklicenses.py (right):

https://codereview.chromium.org/1498473010/diff/120001/tools/checklicenses/ch...
tools/checklicenses/checklicenses.py:68: 'GPL (v3 or later) LGPL (v2.1 or later)
with Bison parser exception',
On 2016/02/09 19:07:52, Lei Zhang wrote:
> Alphabetical order please. Best to sort with LANG=C.

Done.

[Lei Zhang, 2016-02-10 20:19:35]

lgtm

For later - please note with a shared library, there's an extra .so file, and
that needs to be packaged up somehow for ChromeOS.

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/README...
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/120001/third_party/wds/README...
third_party/wds/README.chromium:3: Version: 0.1.0
On 2016/02/10 11:34:37, Mikhail wrote:
> On 2016/02/09 19:07:52, Lei Zhang wrote:
> > Should their be a version bump? Patch set 3 says it updated the version
> number.
> 
> Right, thanks for pointing out.

Woohoo, 1.0! \o/

[Mikhail, 2016-02-11 11:56:09]

Description was changed from

==========
third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the
'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within
sandbox).

BUG=242107
==========

to

==========
third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the
'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within
sandbox).

BUG=242107
==========

[Mikhail, 2016-02-11 11:56:09]

mikhail.pozdnyakov@intel.com changed reviewers:
+ thakis@chromium.org

[Mikhail, 2016-02-11 11:57:27]

@Nico, could you pls review third_party/ changes?

[Nico, 2016-02-11 13:09:15]

license says "Bad content. Try to upload again." for me

[Nico, 2016-02-11 13:11:43]

lgtm assuming this had legal and security reviews

https://codereview.chromium.org/1498473010/diff/140001/third_party/wds/README...
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/140001/third_party/wds/README...
third_party/wds/README.chromium:6: Security Critical: yes
this had a security review, yes?

[Mikhail, 2016-02-11 13:16:15]

https://codereview.chromium.org/1498473010/diff/140001/third_party/wds/README...
File third_party/wds/README.chromium (right):

https://codereview.chromium.org/1498473010/diff/140001/third_party/wds/README...
third_party/wds/README.chromium:6: Security Critical: yes
On 2016/02/11 13:11:43, Nico wrote:
> this had a security review, yes?
Yeah, this has passed both licensing and security checks (as required at
https://www.chromium.org/developers/adding-3rd-party-libraries)

[Mikhail, 2016-02-11 21:26:28]

mikhail.pozdnyakov@intel.com changed reviewers:
+ ben@chromium.org

[Mikhail, 2016-02-11 21:28:06]

** Presubmit ERRORS **
Missing LGTM from OWNERS of dependencies added to DEPS:
    '+libwds',
@Ben, could you pls have a look at third_party/wds/DEPS ?

[Mikhail, 2016-02-12 10:57:38]

On 2016/02/11 21:28:06, Mikhail wrote:
> ** Presubmit ERRORS **
> Missing LGTM from OWNERS of dependencies added to DEPS:
>     '+libwds',
> @Ben, could you pls have a look at third_party/wds/DEPS ?

Looks like I misunderstood the complaints from the bots and this actually a
presubmit script problem described at
https://code.google.com/p/chromium/issues/detail?id=545323 which is not yet
fixed. So I'm adding 'NOPRESUBMIT=true' to this CL in order to get a workaround
for the bug.

[Mikhail, 2016-02-12 10:59:10]

Description was changed from

==========
third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the
'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within
sandbox).

BUG=242107
==========

to

==========
third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the
'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within
sandbox).

NOPRESUBMIT=true
BUG=242107
==========

[Mikhail, 2016-02-12 10:59:43]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[Mikhail, 2016-02-12 10:59:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1498473010/#ps160001
(title: "Updated the repo path to the real one")

[commit-bot: I haz the power, 2016-02-12 10:59:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1498473010/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1498473010/160001

[commit-bot: I haz the power, 2016-02-12 11:10:58]

Description was changed from

==========
third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the
'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within
sandbox).

NOPRESUBMIT=true
BUG=242107
==========

to

==========
third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the
'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within
sandbox).

NOPRESUBMIT=true
BUG=242107
==========

[commit-bot: I haz the power, 2016-02-12 11:10:59]

Committed patchset #5 (id:160001)

[Lei Zhang, 2016-02-12 17:48:17]

On 2016/02/12 10:57:38, Mikhail wrote:
> On 2016/02/11 21:28:06, Mikhail wrote:
> > ** Presubmit ERRORS **
> > Missing LGTM from OWNERS of dependencies added to DEPS:
> >     '+libwds',
> > @Ben, could you pls have a look at third_party/wds/DEPS ?
> 
> Looks like I misunderstood the complaints from the bots and this actually a
> presubmit script problem described at
> https://code.google.com/p/chromium/issues/detail?id=545323 which is not yet
> fixed. So I'm adding 'NOPRESUBMIT=true' to this CL in order to get a
workaround
> for the bug.

You could have avoided it if you followed my advice about not adding a DEPS
file. *shrug*

[commit-bot: I haz the power, 2016-02-16 22:42:39]

Description was changed from

==========
third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the
'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within
sandbox).

NOPRESUBMIT=true
BUG=242107
==========

to

==========
third_party: Add WDS library

This patch adds WDS library into third parties for Linux (incl. CrOS).
The libwds library will be used by the Wi-fi Display implemantation of the
'chrome.displaySource' API on CrOS.
The added library will be called from renderer process only (i.e. within
sandbox).

NOPRESUBMIT=true
BUG=242107

Committed: https://crrev.com/6d56dcc319b7c41dacc1fb0977b8980c301f071b
Cr-Commit-Position: refs/heads/master@{#375159}
==========

[commit-bot: I haz the power, 2016-02-16 22:42:40]

Patchset 5 (id:??) landed as
https://crrev.com/6d56dcc319b7c41dacc1fb0977b8980c301f071b
Cr-Commit-Position: refs/heads/master@{#375159}