third_party/protobuf/README.chromium - Issue 1842653006: Update //third_party/protobuf to version 3.

Side by Side Diff: third_party/protobuf/README.chromium

Issue 1842653006: Update //third_party/protobuf to version 3. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: update defines Created 4 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 Name: Protocol Buffers	1 Name: Protocol Buffers

2 Short Name: protobuf	2 Short Name: protobuf

3 URL: http://protobuf.googlecode.com/svn/trunk	3 URL: https://github.com/google/protobuf

4 License: BSD	4 License: BSD

5 License File: COPYING.txt	5 License File: LICENSE

6 Version: unknown	6 Version: 3.0.0-beta-2

7 Revision: r476	7 Revision: d5fb408ddc281ffcadeb08699e65bb694656d0bd

8 Security Critical: yes	8 Security Critical: yes

9	9

10 Local files (not taken from upstream):	10 Steps used to create the current version:

11 README.chromium	11 1. Pull the release from https://github.com/google/protobuf/releases

12 config.h	12 2. Use the cleanup_release.sh script to remove unnecessary files.

13 descriptor2_pb.py	13 3. Add build files (BUILD.gn, proto_library.gni, protobuf.gyp,

	14 protobuf_lite.gypi, protobuf_nacl.gyp).
	Peter Kasting 2016/04/13 02:11:44 One thing I'm concerned has been lost between my p One thing I'm concerned has been lost between my patch and yours is that mine actually patched the build files along with the source files, and included instructions about what should work after each successive patch. What this meant is that the initial set of buildfiles contained a more direct correspondence to the upstream files, and then as we applied e.g. the componentization patch it was clear exactly what pieces of the build file had to do with that. This would let a future maintainer have more confidence that they understood what in the build files should and should not be carried forward, because the sequential process to construct them was clear. Now we basically just say "add the build files" with no instructions about how to do so, increasing the likelihood that people in the future simply blindly copy the previous buildfiles in totality, even when that wouldn't be correct. xyzzyz 2016/04/14 00:23:56 I'll update the comment to not imply to copy them Show quoted text On 2016/04/13 02:11:44, Peter Kasting wrote: > One thing I'm concerned has been lost between my patch and yours is that mine > actually patched the build files along with the source files, and included > instructions about what should work after each successive patch. What this > meant is that the initial set of buildfiles contained a more direct > correspondence to the upstream files, and then as we applied e.g. the > componentization patch it was clear exactly what pieces of the build file had to > do with that. This would let a future maintainer have more confidence that they > understood what in the build files should and should not be carried forward, > because the sequential process to construct them was clear. > > Now we basically just say "add the build files" with no instructions about how > to do so, increasing the likelihood that people in the future simply blindly > copy the previous buildfiles in totality, even when that wouldn't be correct. I'll update the comment to not imply to copy them blindly, but apart from that, I don't think we can predict all issues with upstream requiring updates, so we should just hope that the future maintainer will have good judgement. I don't think it's worth keeping the diffs to build files, as one can always use git history and code review comments to learn some more background.
	15 4. Get open-source library six.py from https://pypi.python.org/pypi/six/ and add

	16 it to protobuf/third_party/six/six.py.

	17 5. Apply patches in patches/ (see the description below):

14	18

15 A protobuf.gyp file has been added for building with Chromium.	19 $ for patch in patches/*; do patch -s -p1 < $patch; done

16	20

17 This code has been patched to support unknown field retention in protobuf-lite.	21 For future releases, it will be worth looking into which patches still need

18 See r62331 for the patch.	22 to be applied.

	23 5. Generate descriptor_pb2.py using something like the following steps. Make

	24 sure you've regenerated your buildfiles and will build protoc from the

	25 newly-modified sources above.

19	26

20 This code has been patched to ensure that files in the target protobuf_lite	27 $ cd $SRC_DIR

21 do not include headers from protobuf_full. See r173228 for the patch.	28 $ ninja -C out/Debug protoc

	29 $ cd third_party/protobuf/src

	30 $ ../../../out/Debug/protoc --python_out=../python google/protobuf/descriptor .proto

22	31

23 This code has been patched to make the target protobuf_lite a component so that	32 Copy the resulting descriptor_pb2.py to protobuf/.
	Peter Kasting 2016/04/13 02:11:44 This line is inaccurate and should simply be remov This line is inaccurate and should simply be removed. xyzzyz 2016/04/14 00:23:56 Done. Show quoted text On 2016/04/13 02:11:44, Peter Kasting wrote: > This line is inaccurate and should simply be removed. Done.
24 targets that depend on it can be componentized. See http://crbug.com/172800 for	33 6. Copy __init__.py to protobuf/ (it adds third_party/six/ to Python path).
	Peter Kasting 2016/04/13 02:11:44 Copy it from where? Copy it from where? xyzzyz 2016/04/14 00:23:56 I'm not sure how to best express that one should j Show quoted text On 2016/04/13 02:11:44, Peter Kasting wrote: > Copy it from where? I'm not sure how to best express that one should just copy the one I prepared and put in third_party/protobuf. PTAL.
25 details, and r179806 for the patch.	34 7. Update README.chromium.

26	35

27 Revision 504 was cherry-picked from upstream.	36 Description of the patches:

28 Revision 512 was cherry-picked from upstream.

29 Revision 516 was cherry-picked from upstream.

30 Revision 517 was cherry-picked from upstream.

31 Revision 522 was cherry-picked from upstream.

32 Revision 523 was cherry-picked from upstream.

33 Revision 524 was cherry-picked from upstream.

34 Revision 573 was cherry-picked from upstream.

35 The `&file->options() != NULL &&` was removed from descriptor.cc

36 References to `vector` were changed to `std::vector` in extension_set.h

37 and unknown_field_set.h

38	37

39 GetEmptyString() and GoogleOnceInit() have been uninlined, for a large savings	38 - 0001-ignore-option-retain-unknown-fields.patch

40 in binary size.

41	39

42 A BUILD.gn file has been added for building with GN.	40 Previous versions of protobuf in Chromium carried a local patch that retained

	41 unknown fields in protobuf_lite mode. It was enabled by setting option

	42 retain_unknown_fields = true in .proto file. Now that it is enabled by

	43 default, this option is no longer recognized by protobuf, and so I had to

	44 patch it so that I don't have to fix all .proto files in Chromium in a single

	45 CL.

43	46

44 Cherry-pick pherl changes to make protobuf build on VS2015.	47 I plan to remove those occurences, and then this patch will no longer be

	48 necessary.

45	49

46 Cherry-pick c3cb53b (fix "sometimes-uninitialized" warning).	50 - 0002-remove-support-for-unnecessary-languages.patch

47	51

48 Cherry-pick https://github.com/google/protobuf/commit/56a90a2081379a5	52 This patch removes support for C#, Ruby, Objective-C, JS and other languages n ot

	53 used in Chromium from protoc, to decrease the package size.

49	54

50 Cherry-pick https://github.com/google/protobuf/commit/69d660b39ceabea	55 - 0003-remove-static-initializers.patch

51	56

52 Cherry-pick https://github.com/google/protobuf/commit/a74e912a8be1274	57 This patch removes all static initializers from Chromium. The change in Status

	58 class is not completely compatible with upstream, but it's compatible enough

	59 to work in Chromium, which doesn't use this functionality yet.
	Peter Kasting 2016/04/13 02:11:44 Nit: If we land with this patch, it should refer t Nit: If we land with this patch, it should refer to the upstream issue where the correct fix is debated.
	60

	61 - 0004-fix-integer-types-and-shared-library-exports.patch

	62

	63 This patch makes protobuf int64 to be int64_t (as opposed to long long in

	64 upstream), and similarly for other integer types. It also allows exporting

	65 protobuf symbols in Linux .so libraries.
	Peter Kasting 2016/04/13 02:11:44 Why are the int64_t and componentization changes c Why are the int64_t and componentization changes combined in one patch? Those are independent things. The componentization patch should talk about converting to a component and refer to http://crrev.com/179806 , since that CL and associated bug provide a lot of useful context for those changes; see my patch's README. xyzzyz 2016/04/14 00:23:56 They're in a single patch, because I didn't know h Show quoted text On 2016/04/13 02:11:44, Peter Kasting wrote: > Why are the int64_t and componentization changes combined in one patch? Those > are independent things. > > The componentization patch should talk about converting to a component and refer > to http://crrev.com/179806 , since that CL and associated bug provide a lot of > useful context for those changes; see my patch's README. They're in a single patch, because I didn't know how to easily separate it into two patches, and I didn't think it was really worth spending time on it. I'll update the comment to talk about componentization. Peter Kasting 2016/04/15 01:12:59 Do they actually depend on each other? I just ass Show quoted text On 2016/04/14 00:23:56, xyzzyz wrote: > On 2016/04/13 02:11:44, Peter Kasting wrote: > > Why are the int64_t and componentization changes combined in one patch? Those > > are independent things. > > > > The componentization patch should talk about converting to a component and > refer > > to http://crrev.com/179806 , since that CL and associated bug provide a lot of > > useful context for those changes; see my patch's README. > > They're in a single patch, because I didn't know how to easily separate it into > two patches, and I didn't think it was really worth spending time on it. Do they actually depend on each other? I just assumed they touched the same files, in which case it's easy enough to just make two sequential patches that will apply in some particular order with the relevant pieces. The patch is small, so it's probably not a big deal, it's mostly an issue if in the future e.g. upstream accepts the int64_t changes, in which case part of the patch will still be needed and part won't.
	66

	67 - 0005-fix-no-threadlocal-on-mac.patch

	68

	69 Apple's Clang doesn't support thread-local variables on Mac.

	70

	71 - 0006-fix-clang-type-errors.patch

	72

	73 New versions of Clang have type checking semantics that check the conformance

	74 with the C++ standard more thorougly, which makes them a bit more

	75 restrictive. This is actually cherry-pick from protobuf upstream.
	Peter Kasting 2016/04/13 02:11:44 If you've cherry-picked, say exactly which revisio If you've cherry-picked, say exactly which revision you cherry-picked. xyzzyz 2016/04/14 00:23:56 Done. Show quoted text On 2016/04/13 02:11:44, Peter Kasting wrote: > If you've cherry-picked, say exactly which revision you cherry-picked. Done.
	76

	77 - 0007-uninline_googleonce.patch

	78 - 0008-uninline_get_empty_string.patch

	79 - 0009-uninline-arenastring.patch

	80 - 0010-uninline-generated-code.patch

	81

	82 These patches uninline some functions, resulting in a significant reduction

	83 (somewhere between 500 KB and 1 MB) of binary size.

OLD	NEW

« third_party/protobuf/BUILD.gn ('K') | « third_party/protobuf/LICENSE ('k') | third_party/protobuf/README.md » ('j') | third_party/protobuf/protobuf_lite.gypi » ('J')