Skip to content

WIP: Chromium bump #26200

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
Closed
wants to merge 2 commits into from
Closed

WIP: Chromium bump #26200

wants to merge 2 commits into from
+10 −10

Conversation

ryantrinkle
Copy link
Contributor

Motivation for this change

Chromium doesn't seem to build without b746b4e

I could use some help figuring out how to properly test this.

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
    • Linux
  • 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.

Sorry, something went wrong.

Ryan Trinkle added 2 commits May 28, 2017 22:38
chromium: don't sed pdfium past version 58
b746b4e 
In chromium >= 52, we modified third_party/pdfium/xfa/fxbarcode/utils.h in-place using sed; however, in version 58, that file disappeared
chromium: update
2c7c3e1
@mention-bot
Copy link

@ryantrinkle, thanks for your PR! By analyzing the history of the files in this pull request, we identified @joachifm, @aszlig and @abbradar to be potential reviewers.

@pSub pSub added the 2.status: work-in-progress This PR isn't done label May 29, 2017
Mic92
Mic92 reviewed May 29, 2017
View reviewed changes
pkgs/applications/networking/browsers/chromium/common.nix
@@ -130,7 +130,7 @@ let
}' gpu/config/gpu_control_list.cc
patchShebangs .
'' + optionalString (versionAtLeast version "52.0.0.0") ''
'' + optionalString (versionAtLeast version "52.0.0.0" && !(versionAtLeast version "58.0.0.0")) ''
Copy link
Member

@Mic92 Mic92 May 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

optionalString (versionAtLeast version "52.0.0.0" && versionOlder version "58.0.0.0")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@ryantrinkle
Copy link
Contributor Author

Looks like this still fails to build, now with:

[505/26989] ACTION //chrome/browser/resources/md_downloads:vulcanize(//build/toolchain/linux:x64)
FAILED: gen/chrome/browser/resources/md_downloads/vulcanized.unbuilt.html gen/chrome/browser/resources/md_downloads/crisper.js 
python ../../chrome/browser/resources/vulcanize_gn.py --host downloads --html_in_file downloads.html --html_out_file vulcanized.unbuilt.html --js_out_file crisper.js --input ../../chrome/browser/resources/md_downloads --out_folder gen/chrome/browser/resources/md_downloads --depfile gen/chrome/browser/resources/md_downloads/vulcanized.unbuilt.html.d --insert_in_head \<base\ href=chrome://downloads\>
../../third_party/node/linux/node-linux-x64/bin/node ../../third_party/node/node_modules/vulcanize/bin/vulcanize --exclude chrome://resources/html/polymer.html --exclude web-animations-next-lite.min.js --exclude load_time_data.js --exclude strings.js --exclude text_defaults.css --exclude text_defaults_md.css --inline-css --inline-scripts --strip-comments --redirect "chrome://resources/cr_elements/|../../ui/webui/resources/cr_elements" --redirect "chrome://resources/css/|../../ui/webui/resources/css" --redirect "chrome://resources/html/|../../ui/webui/resources/html" --redirect "chrome://resources/js/|../../ui/webui/resources/js" --redirect "chrome://resources/polymer/v1_0/|../../third_party/polymer/v1_0/components-chromium" --out-request-list /tmp/nix-build-chromium-59.0.3071.71.drv-0/chromium-59.0.3071.71/out/Release/gen/chrome/browser/resources/md_downloads/vulcanized.unbuilt.html_requestlist.txt --redirect "/|/tmp/nix-build-chromium-59.0.3071.71.drv-0/chromium-59.0.3071.71/chrome/browser/resources/md_downloads" --redirect "chrome://downloads/|/tmp/nix-build-chromium-59.0.3071.71.drv-0/chromium-59.0.3071.71/chrome/browser/resources/md_downloads" /downloads.html failed: /bin/sh: ../../third_party/node/linux/node-linux-x64/bin/node: No such file or directory

Traceback (most recent call last):
  File "../../chrome/browser/resources/vulcanize_gn.py", line 207, in <module>
    main(sys.argv[1:])
  File "../../chrome/browser/resources/vulcanize_gn.py", line 202, in main
    _vulcanize(args.input, args)
  File "../../chrome/browser/resources/vulcanize_gn.py", line 150, in _vulcanize
    args.html_in_file)])
  File "../../third_party/node/node.py", line 28, in RunNode
    raise
TypeError: exceptions must be old-style classes or derived from BaseException, not NoneType
ninja: build stopped: subcommand failed.

@joachifm
Copy link
Contributor

See also #25940, which claims to fix the build.

@ryantrinkle
Copy link
Contributor Author

@joachifm Ah, ok, cool; that definitely looks like it fixes the new error I was getting. Perhaps I picked the wrong version as the cutoff for the utils.h stuff, or perhaps it's unnecessary for some other reason. I'll look into this more if this issue is run into again.

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

@Mic92 Mic92

Assignees
No one assigned
Labels
2.status: work-in-progress This PR isn't done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ryantrinkle @mention-bot @joachifm @Mic92 @pSub