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.

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 Windows support (continuation) #345

Closed
Eloston opened this issue Mar 3, 2018 · 14 comments
Closed

Update Windows support (continuation) #345

Eloston opened this issue Mar 3, 2018 · 14 comments
Milestone

Comments

@Eloston
Copy link
Member

Eloston commented Mar 3, 2018

For some reason, #215 causes GitHub's Desktop webpage to crash with Server Error 500, but not their Mobile version. Since this is a bit inconvenient for everyone, this issue is a continuation of the discussion in #215.

Just in case #215 breaks completely, an archive of the Mobile version is here: https://gist.github.com/Eloston/24dab695470deba4c20a0b5683e160b4

@mikerockett
Copy link

Out of curiosity, does the decision to move to Clang help this issue in any way?

https://mspoweruser.com/googles-chrome-ditches-microsofts-compiler-clang-windows/

@Eloston
Copy link
Member Author

Eloston commented Mar 6, 2018

@mikerockett The current state is described here: #332 (comment). Basically, the Windows build almost works, except for some Windows-specific code that broke due to GN configuration (which I suspected in the linked comment to be caused by disabling Safe Browsing).

The move to Clang actually slowed me down a bit, because I didn't want to have to setup yet another tool to build Chromium (for purists, this potentially means yet another tool to build from source). In the end, I gave in because some non-trivial problems occurred with the Microsoft compiler.

If anyone's interested, BUILDING.md contains the steps and some notes I used to build Chromium up to the point of failure. I think all it takes to make a working build is to patch out some more obscure references to Safe Browsing-related components. But, there may also be something wrong with the changes I made to the GN files. Faster said than done for me at least.

(A bit unrelated, but I found it interesting that the Chrome Cleaner (downloads and runs the Chrome Cleanup Tool) is implemented within Safe Browsing. Safe Browsing's been more than just a website blacklisting service for a while now, but this is pushing the boundaries even further.)

@mikerockett
Copy link

I see. I’m quite unversed when it comes to much of this, but a bit of insight is always good, so thanks for that. 👍

@Eloston
Copy link
Member Author

Eloston commented Mar 7, 2018

@mercerius (This is a response to #235 (comment))

I forgot to mention that I disabled the 260 character limit for file paths. I'll add a note about that to the BUILDING instructions. See edits

EDIT: I just realized you're using Windows 7. In that case, I'm not sure if Python 3 is the only tool affected by this problem, or if there's a way to use \\?\ in Python scripts.
EDIT2: It seems to be possible to use \\?\, but it's a pain to implement properly. Even Python didn't want to deal with it (Python issue 18199). You could try skipping domain substitution and see if the build will break before the failure point I had. If it does not, then we could add some workarounds to buildkit to accomodate this.

@Eloston
Copy link
Member Author

Eloston commented Mar 12, 2018

Status update for version 65: With #351, the Safe Browsing patch needs to be refreshed.

@Eloston
Copy link
Member Author

Eloston commented Mar 17, 2018

@squalus With #356, do you still get the same problem that I got?

@squalus
Copy link
Member

squalus commented Mar 19, 2018

@Eloston Which problem - the path limit thing? No, I never hit that one. I saw your build doc and took your suggestion of using official Python3.6 binaries and I had a fully updated Win10 system

@Eloston
Copy link
Member Author

Eloston commented Mar 19, 2018

@squalus

Which problem - the path limit thing? No, I never hit that one. I saw your build doc and took your suggestion of using official Python3.6 binaries and I had a fully updated Win10 system

At that time, I was wondering what caused your build to fail. But you have solved that already, so yay!

(Following quotes are from #359 (comment) )

build.bat script did not copy the args.gn file. Don't know why yet. Windows batch files are awful!

Hmm, maybe %~dp0 didn't resolve correctly?

quilt needed the series file to be in unix rather than windows (CRLF) format. Though this may have been because I was using WSL instead of MSYS2. I'll need to check this

I believe MSYS2's git does not convert the newlines automatically to CRLF during cloning (the original files are already in LF). I don't know about the version of git you're using.

build.bat needs to have gperf/bison env vars set

Noted in build.bat

I needed to run the "x64 Native Tools" command prompt rather than use the entries in build.bat. Not sure why

That is strange, since invoking one of them in an interactive command prompt window sets the variables for me. Not sure what's happening here.

LLVM needs to be copied into tree (I have a patch coming to do this in getsrc)

Are you doing this through the extra dependencies mechanism? I would rather extend that than use some hack in buildkit.

EDIT: I'm wondering if it's possible to download gperf and bison this way from the SourceForge links through extra dependencies.

No packaging or installer at the end. I tried using ninja mini-installer but the installer did not start up. Haven't looked into it yet.

I was thinking about using resources/packaging/shared/list_build_outputs.py to determine what files would be added to a zip file. If it's easier, we can write a small Python script to do this rather than use a batch script.

Also, I used the 64 bit LLVM 6.0.0 official binaries from llvm.org

Noted in aa972bd

@squalus
Copy link
Member

squalus commented Mar 19, 2018

LLVM needs to be copied into tree (I have a patch coming to do this in getsrc)

Are you doing this through the extra dependencies mechanism? I would rather extend that than use some hack in buildkit.

I used a hack in buildkit. I have a PR in #360. You probably won't like it but we can discuss :)

No packaging or installer at the end. I tried using ninja mini-installer but the installer did not start up. Haven't looked into it yet.

I was thinking about using resources/packaging/shared/list_build_outputs.py to determine what files would be added to a zip file. If it's easier, we can write a small Python script to do this rather than use a batch script.

Yes I think the linux_simple approach would be great here. I prefer Python (or really anything that's not Windows batch files). Maybe in this case the same script could be shared between the platforms.

@Eloston
Copy link
Member Author

Eloston commented Mar 19, 2018

@squalus

Maybe in this case the same script could be shared between the platforms.

There'll be some performance penalty using Python's implementation (e.g. Python bottlenecks the Chromium source tree unpacking), but it shouldn't be too bad I suppose. I suppose I didn't make the most rational decision here. I can refactor that script to do this.

Also, would you be able to test if %VS140COMNTOOLS% is available in regular command-prompt (or similar variable)? It'll save some path hardcoding (info from https://stackoverflow.com/questions/18711595/how-run-clang-from-command-line-on-windows).

@squalus
Copy link
Member

squalus commented Mar 19, 2018

Also, would you be able to test if %VS140COMNTOOLS% is available in regular command-prompt (or similar variable)? It'll save some path hardcoding (info from https://stackoverflow.com/questions/18711595/how-run-clang-from-command-line-on-windows).

I just checked and it's not available, nor is anything else that looks similar. A search indicates that as of VS2017 they no longer provide such a variable. It seems the newer way to do it is to use this tool: https://github.com/Microsoft/vswhere

@Eloston
Copy link
Member Author

Eloston commented Mar 19, 2018

@squalus So does %ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe exist?

@squalus
Copy link
Member

squalus commented Mar 19, 2018

Yeah, it works.

"%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe" -latest -property installationPath

produces:

C:\Program Files (x86)\Microsoft Visual Studio\2017\Community

@Eloston
Copy link
Member Author

Eloston commented Mar 28, 2018

Huge thanks to @squalus for fixing, developing, and testing Windows support, as well as refactoring the build process into something much more user-friendly (the changes of which have benefitted other platforms too).

Discussion of a Windows build is taking place in #352 (for now).

@Eloston Eloston closed this as completed Mar 28, 2018
@Eloston Eloston added this to the 65.x.x.x milestone Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants