Standalone GN build.

Standalone GN build.

This CL setups up a standalone GN build. You'll need to set gn args similar to:

use_goma = true
clang_use_chrome_plugins = false
pdf_enable_xfa = true
pdf_enable_v8 = true
pdf_is_standalone = true

The third_party/pymock files are needed to make git cl upload work correctly.

BUG=pdfium:106
Committed: https://pdfium.googlesource.com/pdfium/+/685bb88433b7520946e30a1e520d37bfc2288151

[dsinclair, 2016-04-19 21:05:37]

dsinclair@chromium.org changed reviewers:
+ thakis@chromium.org, thestig@chromium.org, tsepez@chromium.org

[dsinclair, 2016-04-19 21:05:37]

PTAL.

[Tom Sepez, 2016-04-19 21:45:10]

lgtm

https://codereview.chromium.org/1904563002/diff/20001/DEPS
File DEPS (right):

https://codereview.chromium.org/1904563002/diff/20001/DEPS#newcode110
DEPS:110: 'action': ['python',
'pdfium/build/linux/sysroot_scripts/install-sysroot.py',
nit: format one per line, closing "]" lines up under 'action' as above.

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README.md
File build_overrides/README.md (right):

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:6: For example: V8 could be built on its own (in a
"standalone" configuration),
Nit: might prefer "can" over "could" and adjust grammar all the way down.

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:7: and it could be built as part of Chromium. V8 might
define a top-level
s/might define/defines/

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:9: standalone configuration. To figure out whether or
not it should be
To indicate that it is a standalone configuation

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:15: Chromium, on the other hand, might not need to
build d8, and so it would
s/might not/does not/

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:19: The two files should define the same set of
variables, but the values can
should vs. must ?

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:20: vary as appropriate to the needs of the two
different builds.
to suit the needs

https://codereview.chromium.org/1904563002/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/1904563002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:207: for (size_t i = 0; i <
m_DirectObjs.size(); ++i) {
Is this a stray file from some other cleanup?  No matter.

https://codereview.chromium.org/1904563002/diff/20001/core/fxcrt/fx_basic_wst...
File core/fxcrt/fx_basic_wstring.cpp (right):

https://codereview.chromium.org/1904563002/diff/20001/core/fxcrt/fx_basic_wst...
core/fxcrt/fx_basic_wstring.cpp:582: const FX_WCHAR* CFX_WideString::c_str()
const {
same here

[dsinclair, 2016-04-20 03:06:51]

Description was changed from

==========
Standalone GN build.

This CL setups up a standalone GN build. You'll need to set gn args similar to:

use_goma = true
clang_use_chrome_plugins = false
pdf_enable_xfa = true
pdf_enable_v8 = true
pdf_is_standalone = true

The third_party/pymock files are needed to make git cl upload work correctly.

BUG=pdfium:106
==========

to

==========
Standalone GN build.

This CL setups up a standalone GN build. You'll need to set gn args similar to:

use_goma = true
clang_use_chrome_plugins = false
pdf_enable_xfa = true
pdf_enable_v8 = true
pdf_is_standalone = true

The third_party/pymock files are needed to make git cl upload work correctly.

BUG=pdfium:106
==========

[dsinclair, 2016-04-20 03:06:51]

dsinclair@chromium.org changed reviewers:
+ weili@chromium.org

[dsinclair, 2016-04-20 13:19:39]

https://codereview.chromium.org/1904563002/diff/20001/DEPS
File DEPS (right):

https://codereview.chromium.org/1904563002/diff/20001/DEPS#newcode110
DEPS:110: 'action': ['python',
'pdfium/build/linux/sysroot_scripts/install-sysroot.py',
On 2016/04/19 21:45:09, Tom Sepez wrote:
> nit: format one per line, closing "]" lines up under 'action' as above.

Done.

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README.md
File build_overrides/README.md (right):

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:6: For example: V8 could be built on its own (in a
"standalone" configuration),
On 2016/04/19 21:45:09, Tom Sepez wrote:
> Nit: might prefer "can" over "could" and adjust grammar all the way down.

Done.

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:7: and it could be built as part of Chromium. V8 might
define a top-level
On 2016/04/19 21:45:09, Tom Sepez wrote:
> s/might define/defines/

Done.

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:9: standalone configuration. To figure out whether or
not it should be
On 2016/04/19 21:45:09, Tom Sepez wrote:
> To indicate that it is a standalone configuation

Done.

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:15: Chromium, on the other hand, might not need to
build d8, and so it would
On 2016/04/19 21:45:09, Tom Sepez wrote:
> s/might not/does not/

Done.

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:19: The two files should define the same set of
variables, but the values can
On 2016/04/19 21:45:09, Tom Sepez wrote:
> should vs. must ?

Sounds like it isn't required (this file is copied from chromium originally)

https://codereview.chromium.org/1904563002/diff/20001/build_overrides/README....
build_overrides/README.md:20: vary as appropriate to the needs of the two
different builds.
On 2016/04/19 21:45:09, Tom Sepez wrote:
> to suit the needs

Done.

https://codereview.chromium.org/1904563002/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/1904563002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:207: for (size_t i = 0; i <
m_DirectObjs.size(); ++i) {
On 2016/04/19 21:45:09, Tom Sepez wrote:
> Is this a stray file from some other cleanup?  No matter.

This test failed due to the inlined c_str, I fixed this bit when I was working
out why the test broke.

https://codereview.chromium.org/1904563002/diff/20001/core/fxcrt/fx_basic_wst...
File core/fxcrt/fx_basic_wstring.cpp (right):

https://codereview.chromium.org/1904563002/diff/20001/core/fxcrt/fx_basic_wst...
core/fxcrt/fx_basic_wstring.cpp:582: const FX_WCHAR* CFX_WideString::c_str()
const {
On 2016/04/19 21:45:09, Tom Sepez wrote:
> same here

When I did this conversion the first time, having this in the header caused the
test to fail. Moving the implementation here fixed the issue.

[Nico, 2016-04-20 13:29:29]

lgtm, but I'd do this in more CLs (one CL for the build->build_gyp rename, it'd
probably include depsing in build; then one for getting a minimal gn build
going, then one to actually build tests, or something like that)

https://codereview.chromium.org/1904563002/diff/40001/build_overrides/build.gni
File build_overrides/build.gni (right):

https://codereview.chromium.org/1904563002/diff/40001/build_overrides/build.g...
build_overrides/build.gni:9: # requires 10.7.
chromium dropped 10.6 a while ago. I think you don't need the
target_build_override, but if you need it, set it to 10.7 at least, and possibly
to 10.9 (where chrome will be eventually)

[dsinclair, 2016-04-20 13:35:07]

https://codereview.chromium.org/1904563002/diff/40001/build_overrides/build.gni
File build_overrides/build.gni (right):

https://codereview.chromium.org/1904563002/diff/40001/build_overrides/build.g...
build_overrides/build.gni:9: # requires 10.7.
On 2016/04/20 13:29:28, Nico wrote:
> chromium dropped 10.6 a while ago. I think you don't need the
> target_build_override, but if you need it, set it to 10.7 at least, and
possibly
> to 10.9 (where chrome will be eventually)

Done.

[dsinclair, 2016-04-20 13:36:29]

On 2016/04/20 13:29:29, Nico wrote:
> lgtm, but I'd do this in more CLs (one CL for the build->build_gyp rename,
it'd
> probably include depsing in build; then one for getting a minimal gn build
> going, then one to actually build tests, or something like that)
> 

The build -> build_gyp change is done. I'd left the two files thinking I needed
them for the bots, but the bots do the gyp_pdfium through gclient and I'd
already updated DEPS.

All the GN files to build PDFium already exist as we work with the Chrome GN
build, so wouldn't I need to remove functionality to get just tests building?

[Nico, 2016-04-20 13:39:24]

Huh, adding pymock and whatnot seems unnecessary to me for just moving
pdfium-local files out of the build/ folder. If it isn't for some reason I don't
see, then ignore my comment.

[dsinclair, 2016-04-20 13:43:04]

On 2016/04/20 13:39:24, Nico wrote:
> Huh, adding pymock and whatnot seems unnecessary to me for just moving
> pdfium-local files out of the build/ folder. If it isn't for some reason I
don't
> see, then ignore my comment.

Moving stuff out of build/ was done in
https://codereview.chromium.org/1900913003/. This Cl is hooking up the bits of
for GN standalone to work.

[Nico, 2016-04-20 13:44:57]

This change looks like it deletes build/gyp_pdfium. Maybe the CL wasn't rebased
after that other change landed?

[dsinclair, 2016-04-20 13:46:38]

On 2016/04/20 13:44:57, Nico wrote:
> This change looks like it deletes build/gyp_pdfium. Maybe the CL wasn't
rebased
> after that other change landed?


In the other CL, I left build/gyp_pdfium thinking I'd need an intermediate CL to
update the bots. When I looked, I found the bots ran gyp_pdfium through gclient,
so the DEPS change in the last CL covered it.

Instead of doing a Cl to just remove the last two files, I combined it with this
CL. The gyp_pdfium files currently exist in both build/ and build_gyp/ before
this CL.

[Nico, 2016-04-20 13:47:43]

right, that's the bit i meant i'd do in a separate cl :-) (since that bit might
affect bots, and the rest probably doesn't since there are no standalone gn bots
yet). but if you want to not split it, you have my lg already.

[dsinclair, 2016-04-20 13:50:30]

On 2016/04/20 13:47:43, Nico wrote:
> right, that's the bit i meant i'd do in a separate cl :-) (since that bit
might
> affect bots, and the rest probably doesn't since there are no standalone gn
bots
> yet). but if you want to not split it, you have my lg already.

Ah, sorry, that makes sense. https://codereview.chromium.org/1908433003/

[dsinclair, 2016-04-20 14:15:03]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-04-20 14:15:03]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1904563002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2016-04-20 14:15:13]

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

[commit-bot: I haz the power, 2016-04-20 14:32:42]

Description was changed from

==========
Standalone GN build.

This CL setups up a standalone GN build. You'll need to set gn args similar to:

use_goma = true
clang_use_chrome_plugins = false
pdf_enable_xfa = true
pdf_enable_v8 = true
pdf_is_standalone = true

The third_party/pymock files are needed to make git cl upload work correctly.

BUG=pdfium:106
==========

to

==========
Standalone GN build.

This CL setups up a standalone GN build. You'll need to set gn args similar to:

use_goma = true
clang_use_chrome_plugins = false
pdf_enable_xfa = true
pdf_enable_v8 = true
pdf_is_standalone = true

The third_party/pymock files are needed to make git cl upload work correctly.

BUG=pdfium:106

Committed:
https://pdfium.googlesource.com/pdfium/+/685bb88433b7520946e30a1e520d37bfc228...
==========

[commit-bot: I haz the power, 2016-04-20 14:32:43]

Committed patchset #5 (id:80001) as
https://pdfium.googlesource.com/pdfium/+/685bb88433b7520946e30a1e520d37bfc228...