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.

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

Update Chromium Upgrade docs #10215

Merged
merged 3 commits into from Aug 28, 2017

Conversation

Projects
None yet
4 participants
@alexeykuzmin
Contributor

alexeykuzmin commented Aug 7, 2017

@zeke

zeke approved these changes Aug 10, 2017

Awesome. Thanks for putting this together.

@alexeykuzmin

This comment has been minimized.

Contributor

alexeykuzmin commented Aug 10, 2017

@zeke Thanks for the approve )

But I want it to stay unmerged for a couple of days to have a chance to change wording and
probably add some stuff I forgot.

https://chromium.googlesource.com/chromium/src.git/+/{VERSION}/tools/clang/scripts/update.py
(Just change the `{VERSION}` in the URL to the Chromium version you need.)
- Don't forget to (re)run `script/bootstrap.py`.
Chromium is using. You can find it in file `src/tools/clang/scripts/update.py` in updated `electron/libchromiumcontent` repo.

This comment has been minimized.

@alexeykuzmin

alexeykuzmin Aug 11, 2017

Contributor

I would keep a link to the file in the Chromium repo.
If one is working on Electron update without having the libcc built on the same machine (and uses downloaded binaries instead), it's quite difficult and time consuming to checkout Chromium only to get right clang revision.

This comment has been minimized.

@tonyganch

tonyganch Aug 11, 2017

Member

We should then specify two branches in the docs: if you have libcc locally (including how to symlink it or something), and if you download it from somewhere (current docs don't say how to do that)

to build libcc locally in order to try build a new Electron.
- Set `CLANG_REVISION` in `script/update-clang.sh` to match the version
Chromium is using. You can find it in file `src/tools/clang/scripts/update.py` in updated `electron/libchromiumcontent` repo.
- Run `script/bootstrap.py`.

This comment has been minimized.

@alexeykuzmin

alexeykuzmin Aug 11, 2017

Contributor

Maybe we should mention --libcc_source_path= and other --libcc_... arguments of the bootstrap script?
It might be easier to use them than to create a symlink, especially on Windows.

This comment has been minimized.

@tonyganch

tonyganch Aug 11, 2017

Member

Yes, that will be cool. I couldn't make it work when updating libcc for 61 though, no idea why, that's why i sticked with symlink. Maybe it's worth mentioning both approaches, just in case

```
- Create dist folders which will be used by electron:
```
./script/create-dist --no_zip

This comment has been minimized.

@alexeykuzmin

alexeykuzmin Aug 17, 2017

Contributor

I'm not sure if it's a good idea to use so detailed commands in this guide. I mean it might be hard to keep them up-to-date all the time. Maybe it would make more sense to say what should be done instead of how?, and provide links to libcc/Electron build docs? @tonyganch, what do you think?

This comment has been minimized.

@enlight

enlight Aug 18, 2017

Contributor

Jumping between multiple documents trying figure out the correct sequence of commands and arguments when you're doing this for the first time (or any time after that really) is annoying.

@alexeykuzmin

This comment has been minimized.

Contributor

alexeykuzmin commented Aug 27, 2017

@zeke
I want to merge it as it is now. It's not perfect of course, but at least it's better than the current doc.
I'm going to update it again in couple of weeks, probably after Chromium 61 upgrade.
Are you ok with that?

@zeke zeke changed the title from [WIP] Update Chromium Upgrade docs to Update Chromium Upgrade docs Aug 28, 2017

@zeke zeke merged commit 66a5ac4 into master Aug 28, 2017

0 of 7 checks passed

ci/circleci Your tests are queued behind your running builds
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
electron-linux-arm Build #7788239 queued
Details
electron-linux-arm64 Build #7788240 queued
Details
electron-linux-ia32 Build #7788241 queued
Details
electron-linux-x64 Build #7788242 queued
Details

@zeke zeke deleted the update-chromium-upgrade-docs branch Aug 28, 2017

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