New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 832044 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Move preg_parser out of libchrome and remove it from Chromium

Project Member Reported by ljusten@chromium.org, Apr 12 2018

Issue description

preg_parser isn't used in Chromium anymore, so it can be removed. Only platform2/authpolicy still uses it. It should be removed from Chromium and moved from libchrome to authpolicy.
 
Labels: Pri-1
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/external/libchrome/+/0b087f9dcf23367a24f0bacc738abe9ec8ed6506

commit 0b087f9dcf23367a24f0bacc738abe9ec8ed6506
Author: A Olsen <olsen@chromium.org>
Date: Tue Apr 24 15:39:49 2018

libchrome: Delete preg_parser.cc from libchrome at HEAD, not as a patch

See CL:1016763 which patches the current ebuild version of libchrome to
delete preg_parser, and moves it into authpolicy. This is because
preg_parser is no longer used by chrome or libchrome, authpolicy is the
only remaining user. So this CL makes the same change at HEAD so that
the delete_preg_parser.patch will no longer be needed once libchrome
is upreved to include this CL.

BUG= chromium:832044 
TEST=Build libchrome and inspect it to see it doesn't have preg_parser any more.

Change-Id: I7727965f34fd96688837fbcdcccbf59e1f76d69e
Reviewed-on: https://chromium-review.googlesource.com/1023990
Commit-Ready: A Olsen <olsen@chromium.org>
Tested-by: A Olsen <olsen@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>

[modify] https://crrev.com/0b087f9dcf23367a24f0bacc738abe9ec8ed6506/SConstruct
[delete] https://crrev.com/876a619fbd629f047eec163321fb770cad3e3ea5/components/policy/core/common/preg_parser_unittest.cc
[delete] https://crrev.com/876a619fbd629f047eec163321fb770cad3e3ea5/components/policy/core/common/preg_parser.cc
[delete] https://crrev.com/876a619fbd629f047eec163321fb770cad3e3ea5/components/policy/core/common/preg_parser.h

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/1a9da1071c694364a673b1004b51867fc1b271d6

commit 1a9da1071c694364a673b1004b51867fc1b271d6
Author: A Olsen <olsen@chromium.org>
Date: Tue Apr 24 20:31:14 2018

libchrome: Move preg_parser from libchrome into authpolicy

Moves the .cc, .h, unit test + testdata and fuzzer for preg_parser into
authpolicy/policy, out from libchrome. This is because preg_parser is no
longer used by chrome or libchrome, authpolicy is the only remaining
user.

(This is the libchrome half of a change that touches libchrome and
authpolicy.)

BUG= chromium:832044 
TEST=Build libchrome and inspect it to see it doesn't have preg_parser any more.
CQ-DEPEND=CL:1016605

Change-Id: I1cbbd9f3212a97ded069ffce11d03c0dff04cdef
Reviewed-on: https://chromium-review.googlesource.com/1016763
Commit-Ready: A Olsen <olsen@chromium.org>
Tested-by: A Olsen <olsen@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>

[rename] https://crrev.com/1a9da1071c694364a673b1004b51867fc1b271d6/chromeos-base/libchrome/libchrome-395517-r28.ebuild
[add] https://crrev.com/1a9da1071c694364a673b1004b51867fc1b271d6/chromeos-base/libchrome/files/libchrome-395517-delete-preg-parser.patch
[modify] https://crrev.com/1a9da1071c694364a673b1004b51867fc1b271d6/virtual/chromium-os-fuzzers/chromium-os-fuzzers-1.ebuild
[modify] https://crrev.com/1a9da1071c694364a673b1004b51867fc1b271d6/chromeos-base/libchrome/libchrome-395517.ebuild
[rename] https://crrev.com/1a9da1071c694364a673b1004b51867fc1b271d6/virtual/chromium-os-fuzzers/chromium-os-fuzzers-1-r3.ebuild
[modify] https://crrev.com/1a9da1071c694364a673b1004b51867fc1b271d6/chromeos-base/authpolicy/authpolicy-9999.ebuild

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/223ae34f292996313376707401e7143be8d1aebb

commit 223ae34f292996313376707401e7143be8d1aebb
Author: A Olsen <olsen@chromium.org>
Date: Tue Apr 24 20:31:12 2018

authpolicy: Move preg_parser from libchrome into authpolicy

Moves the .cc, .h, unit test + testdata and fuzzer for preg_parser into
authpolicy/policy, out from libchrome. This is because preg_parser is no
longer used by chrome or libchrome, authpolicy is the only remaining
user.

(This is the authpolicy half of a change that touches libchrome and
authpolicy.)

BUG= chromium:832044 
TEST=cros_run_unit_tests --board=amd64-generic --packages authpolicy
CQ-DEPEND=CL:1016763

Change-Id: I8c262384ef4b79de778147c42ce79dd734c77800
Reviewed-on: https://chromium-review.googlesource.com/1016605
Commit-Ready: A Olsen <olsen@chromium.org>
Tested-by: A Olsen <olsen@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>

[add] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/testdata/preg_parser_fuzzer.dict
[modify] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/authpolicy.gyp
[add] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/preg_parser.h
[add] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/preg_parser_unittest.cc
[modify] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/preg_policy_writer.cc
[add] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/preg_parser_fuzzer.cc
[add] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/testdata/registry.pol
[add] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/preg_parser.cc
[add] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/testdata/preg_parser_fuzzer_seed_corpus.zip
[modify] https://crrev.com/223ae34f292996313376707401e7143be8d1aebb/authpolicy/policy/policy_encoder_helper.cc

Cc: gwendal@chromium.org j...@chromium.org
Hi, I just synced my tree and it looks like removing the shared preg_policy.h header files caused a build failure:

authpolicy-0.0.1-r1093: FAILED: obj/authpolicy/policy/libauthpolicy.policy_encoder_helper.o 
authpolicy-0.0.1-r1093: x86_64-cros-linux-gnu-clang++ -MMD -MF obj/authpolicy/policy/libauthpolicy.policy_encoder_helper.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Iobj/authpolicy/libauthpolicy.gen/include -Igen/include -I/build/eve/tmp/portage/chromeos-base/authpolicy-0.0.1-r1093/work/authpolicy-0.0.1 -I/build/eve/tmp/portage/chromeos-base/authpolicy-0.0.1-r1093/work/platform -I/build/eve/usr/include -Igen -Wshadow -Wextra -Wall -Wno-psabi -Wunused -Wno-unused-parameter -ggdb3 -fstack-protector-strong -Wformat=2 -fvisibility=internal -Wa,--noexecstack -Werror --sysroot=/build/eve -DUSE_RTTI_FOR_TYPE_TAGS -Wno-c++11-extensions -Wno-unused-local-typedefs -DBASE_VER=395517 -pthread -I/build/eve/usr/include/chromeos -I/build/eve/usr/include/base-395517 -I/build/eve/usr/include/glib-2.0 -I/build/eve/usr/lib64/glib-2.0/include -I/build/eve/usr/include/nss -I/build/eve/usr/include/nspr -I/build/eve/usr/include/dbus-1.0 -I/build/eve/usr/lib64/dbus-1.0/include -fPIE -std=gnu++14 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -O2 -pipe -O2 -pipe -O2 -pipe -march=corei7 -g -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -clang-syntax -clang-syntax -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables  -c ../../../../../../../tmp/portage/chromeos-base/authpolicy-0.0.1-r1093/work/authpolicy-0.0.1/authpolicy/policy/policy_encoder_helper.cc -o obj/authpolicy/policy/libauthpolicy.policy_encoder_helper.o
authpolicy-0.0.1-r1093: ../../../../../../../tmp/portage/chromeos-base/authpolicy-0.0.1-r1093/work/authpolicy-0.0.1/authpolicy/policy/policy_encoder_helper.cc:15:10: fatal error: 'components/policy/core/common/preg_parser.h' file not found
authpolicy-0.0.1-r1093: #include <components/policy/core/common/preg_parser.h>
authpolicy-0.0.1-r1093:          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
authpolicy-0.0.1-r1093: 1 error generated.
authpolicy-0.0.1-r1093: ninja: build stopped: subcommand failed.
authpolicy-0.0.1-r1093: 
authpolicy-0.0.1-r1093: cmd=['ninja', '-C', '/build/eve/var/cache/portage/chromeos-base/authpolicy/out/Default', '-j', '40', 'all']
authpolicy-0.0.1-r1093:  * ERROR: chromeos-base/authpolicy-0.0.1-r1093::chromiumos failed (compile phase):
authpolicy-0.0.1-r1093:  *   (no error message)
authpolicy-0.0.1-r1093:  * 
authpolicy-0.0.1-r1093:  * Call stack:
authpolicy-0.0.1-r1093:  *     ebuild.sh, line  133:  Called src_compile
authpolicy-0.0.1-r1093:  *   environment, line 4131:  Called platform_src_compile
authpolicy-0.0.1-r1093:  *   environment, line 3729:  Called platform 'compile' 'all'
authpolicy-0.0.1-r1093:  *   environment, line 3659:  Called die
authpolicy-0.0.1-r1093:  * The specific snippet of code:
authpolicy-0.0.1-r1093:  *       "${cmd[@]}" || die


Guessing this is because the authpolicy ebuild has not been bumped yet?  I ran `cros_workon-eve start authpolicy` and then I was able to emerge the package.
Ahh right, libchrome is uprev'ed manually, but authpolicy uses the auto-uprev. Luckily, there was an auto-uprev about an hour after the code was submitted, so you should be good to go if you sync again. Sorry for the inconvenience!

Comment 7 by olsen@chromium.org, Apr 27 2018

Status: Fixed (was: Assigned)
Fixed: preg_parser is now part of authpolicy library.

To verify:
1. Build libchrome using the latest ebuild:
2. Find the tbz2 file for libchrome, and extract ./usr/lib64/libbase-policy-395517.so:
$ tar -jxvf /build/amd64-generic/packages/chromeos-base/libchrome-395517-r28.tbz2 ./usr/lib64/libbase-policy-395517.so
3. Check for any symbol containing "parser" in it:
$ objdump -TC usr/lib64/libbase-policy-395517.so | grep parser

Nothing found.

If you do the same  thing for the previous ebuild, and extract the policy SO from /build/amd64-generic/packages/chromeos-base/libchrome-395517-r27.tbz2 you will find
some symbols:
policy::preg_parser::ReadFile
policy::preg_parser::kPRegFileHeader
Status: Verified (was: Fixed)
Marked as "Verified" based on c#7.
Project Member

Comment 9 by bugdroid1@chromium.org, May 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/5603f33087107386bdb80d1cc04662a5eb823c2b

commit 5603f33087107386bdb80d1cc04662a5eb823c2b
Author: Manoj Gupta <manojgupta@google.com>
Date: Tue May 01 05:54:22 2018

authpolicy: Update fuzzer files installation.

Switch to the platform_fuzzer_install function for installing
seed corpus and dict files.

CQ-DEPEND=CL:1025969
BUG= chromium:832044 
BUG= chromium:831877 
TEST=No change in installed files with USE="asan fuzzer".

Change-Id: I3bed69c4562aad434d0c267b85369430442803f1
Reviewed-on: https://chromium-review.googlesource.com/1027085
Commit-Ready: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: A Olsen <olsen@chromium.org>

[modify] https://crrev.com/5603f33087107386bdb80d1cc04662a5eb823c2b/chromeos-base/authpolicy/authpolicy-9999.ebuild

Sign in to add a comment