Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chromium: 61.0.3163.100 -> 62.0.3202.62 #30603

Closed
wants to merge 1 commit into from
Closed

chromium: 61.0.3163.100 -> 62.0.3202.62 #30603

wants to merge 1 commit into from

Conversation

YorikSar
Copy link
Contributor

@YorikSar YorikSar commented Oct 20, 2017 •
edited

Motivation for this change

Stable version updated to 62 with various security and other fixes.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@jtojnar
Copy link
Contributor

jtojnar commented Oct 20, 2017 •
edited

See also #30562. It does not remove the patches though.

@YorikSar
Copy link
Contributor Author

@jtojnar that one fails because it doesn't remove them (there's a warning about unneeded patches). And doesn't seem to follow contribution guidelines.

@YorikSar
Copy link
Contributor Author

Ok, it seems not everything builds fine now...

@joachifm joachifm requested a review from bendlas October 20, 2017 13:30
@bendlas
Copy link
Contributor

bendlas commented Oct 20, 2017

@YorikSar if you have a build failure, I recommend retrying with the updated (-r3) version of the gcc5 patch from gentoo: https://gitweb.gentoo.org/repo/gentoo.git/plain/www-client/chromium/

If you could also compare and update the patches for chromium 63, that would be extra nice.

I'll try the builds later, when at home.

thanks

@bendlas bendlas mentioned this pull request Oct 20, 2017
Closed
@YorikSar
Copy link
Contributor Author

@bendlas Thanks, yes, that's my thinking. I'm currently verifying Stable/Beta versions, it takes quite some time to build on my laptop, but so far it seems to be compiling everything just fine.

That said, why are you referring to r3 version of that patch? I thought about using r4 instead. And, surely, I'll pull along all the latest patches from Gentoo repo.

@bendlas
Copy link
Contributor

bendlas commented Oct 20, 2017

@YorikSar when looking at the .ebuild files, you see that -r3 is for 62 and -r4 for 63

@YorikSar
Copy link
Contributor Author

@bendlas, oh, right. For version 62 everything builds fine with r2 version. It's 63 (dev) version that seems to be tricky.

@YorikSar
Copy link
Contributor Author

I've uploaded new version of the commit. Dev version still won't build, it raises these error:

[69/363] g++ -MMD -MF base/metrics/histogram_samples.o.d  -I/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/out_bootstrap/gen -I/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9 -DNO_TCMALLOC -
D__STDC_FORMAT_MACROS -O2 -g0 -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -pthread -pipe -fno-exceptions -std=c++14 -Wno-c++11-narrowing -c /tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.
0.3239.9/base/metrics/histogram_samples.cc -o base/metrics/histogram_samples.o
FAILED: base/metrics/histogram_samples.o
g++ -MMD -MF base/metrics/histogram_samples.o.d  -I/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/out_bootstrap/gen -I/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9 -DNO_TCMALLOC -D__STDC_F
ORMAT_MACROS -O2 -g0 -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -pthread -pipe -fno-exceptions -std=c++14 -Wno-c++11-narrowing -c /tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/
base/metrics/histogram_samples.cc -o base/metrics/histogram_samples.o
In file included from /tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math.h:13:0,
                 from /tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/safe_math.h:8,
                 from /tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/time/time.h:63,
                 from /tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/metrics/histogram_base.h:20,
                 from /tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/metrics/histogram_samples.h:16,
                 from /tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/metrics/histogram_samples.cc:5:
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math_impl.h: In instantiation of 'static constexpr bool base::internal::CheckedSubOp<T, U, typename std::enable_if<(std::is_integral<_Tp
>::value && std::is_integral<_Tp2>::value)>::type>::Do(T, U, V*) [with V = short unsigned int; T = short unsigned int; U = short unsigned int]':
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math.h:245:29:   required from 'constexpr base::internal::CheckedNumeric<T>& base::internal::CheckedNumeric<T>::MathOp(R) [with M = base
::internal::CheckedSubOp; R = short unsigned int; T = short unsigned int]'
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math.h:340:1:   required from 'constexpr base::internal::CheckedNumeric<T>& base::internal::CheckedNumeric<T>::operator-=(R) [with Src =
 short unsigned int; T = short unsigned int]'
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/metrics/histogram_samples.cc:145:20:   required from here
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math_impl.h:130:15: error: uninitialized variable 'presult' in 'constexpr' function
     Promotion presult;
               ^~~~~~~
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math_impl.h: In instantiation of 'static constexpr bool base::internal::CheckedAddOp<T, U, typename std::enable_if<(std::is_integral<_Tp
>::value && std::is_integral<_Tp2>::value)>::type>::Do(T, U, V*) [with V = short unsigned int; T = short unsigned int; U = short unsigned int]':
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math.h:245:29:   required from 'constexpr base::internal::CheckedNumeric<T>& base::internal::CheckedNumeric<T>::MathOp(R) [with M = base
::internal::CheckedAddOp; R = short unsigned int; T = short unsigned int]'
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math.h:339:1:   required from 'constexpr base::internal::CheckedNumeric<T>& base::internal::CheckedNumeric<T>::operator+=(R) [with Src =
 short unsigned int; T = short unsigned int]'
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/metrics/histogram_samples.cc:147:20:   required from here
/tmp/nix-build-chromium-63.0.3239.9.drv-0/chromium-63.0.3239.9/base/numerics/checked_math_impl.h:70:15: error: uninitialized variable 'presult' in 'constexpr' function
     Promotion presult;
               ^~~~~~~
cc1plus: warning: unrecognized command line option '-Wno-c++11-narrowing'

I guess I'll need to investigate further.

@YorikSar
Copy link
Contributor Author

Found a bug in Gentoo about this: https://bugs.gentoo.org/633452, leading to this change: https://chromium-review.googlesource.com/c/chromium/src/+/603127 - from the commit message, we could probably try to revert it...

@YorikSar YorikSar force-pushed the chromium-update branch 2 times, most recently from 3d37e86 to 5f35820 Compare October 20, 2017 23:04
@YorikSar
Copy link
Contributor Author

YorikSar commented Oct 20, 2017 •
edited

Found a fix for it at https://chromium-review.googlesource.com/c/chromium/src/+/730709 - it seems to be good on review.

Now it fails with

[363/363] g++ -pthread -o gn -Wl,--start-group tools/gn/gn_main.o libevent.a base.a xdg_user_dirs.a gn_lib.a dynamic_annotations.a -Wl,--end-group -lrt -latomic
Building gn manually in a temporary directory for bootstrapping...
ERROR Unresolved dependencies.
//third_party:freetype_harfbuzz(//build/toolchain/linux:x64)
  needs //third_party/harfbuzz-ng:harfbuzz_source(//build/toolchain/linux:x64)
//third_party/freetype:freetype_source(//build/toolchain/linux:x64)
  needs //third_party/harfbuzz-ng:harfbuzz_config(//build/toolchain/linux:x64)

builder for ‘/nix/store/7s1258zf68n0v8l5dxmr2sz464cl6vj5-chromium-63.0.3239.9.drv’ failed with exit code 1

Will dig further.

@bendlas
Copy link
Contributor

bendlas commented Oct 21, 2017

I've committed your current patch, because it contains security updates: f0a0f02

Leaving the PR open, for further investigations into the 63 build.
The failure, you're getting looks like a missing gn-bootstrap patch.

@YorikSar
Copy link
Contributor Author

@bendlas no, they've changed how they build freetype and harfbuzz - it seems it doesn't support using one from system and another from source right now. I'll try to come up with solution tomorrow.

@YorikSar
chromium: fix dev build - use harfbuzz from sources
7977c5c 
Unfortunatelly after [0] chromium doesn't support using harfbuzz
provided by system while using vendored version of freetype. Disabling
usage of separate harfbuzz for now.

[0] https://chromium-review.googlesource.com/c/chromium/src/+/696241
@YorikSar
Copy link
Contributor Author

I've disabled using system harfbuzz for dev version for now. I don't have time to wait for it to be built to finish right now though, but it passed main pain points there.

@bendlas Can you also cherry-pick these commits to release-17.09? I'd love to finally see new Chrome on my system (it's funny how to upgrade Chrome in pkgs one has to fight with weirdness of Chromium builds).

@fpletz
Copy link
Member

fpletz commented Oct 25, 2017

For reference, this was backported to 17.09: 527eb2b

@YorikSar
Copy link
Contributor Author

@fpletz @bendlas Thanks. I guess I'll close this PR and open a new one for another version bump that just happened.

@YorikSar YorikSar closed this Oct 27, 2017
@YorikSar
Copy link
Contributor Author

Please see #30863 for new version bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bendlas bendlas Awaiting requested review from bendlas

Assignees
No one assigned
Labels
12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@YorikSar @jtojnar @bendlas @fpletz @Janik-Haag