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

Fix building on Windows. #147

Merged
merged 7 commits into from
Dec 16, 2016
Merged

Fix building on Windows. #147

merged 7 commits into from
Dec 16, 2016

Conversation

nopjmp
Copy link
Contributor

@nopjmp nopjmp commented Dec 15, 2016

This fixes building on Windows. We disable reordering and fix iridium to link correctly on Chromium 55. We still download syzygy for now until we decide if we want to keep it. icu_use_data_file has to be enabled on Windows for now to get it to at least build. Patches are not read into memory and piped over stdin anymore.

We can just kill syzygy building/downloading all together if you like, but I think it may be worth looking into at some point if performance or linking problems come up.

Should fix both #137 #118

This fixes building on Windows. We disable reordering and fix iridium to link correctly on Chromium 55. We still download syzygy for now until we decide if we want to keep it. icu_use_data_file has to be enabled on Windows for now to get it to at least build. Patches are not read into memory and piped over stdin anymore.
@nopjmp
Copy link
Contributor Author

nopjmp commented Dec 15, 2016

I do apologize for the terribly formatted git message. I was in a rush to get this pull request created before I went to bed to avoid another 24 hour wait.

I'll check 64bit build tomorrow.

@Eloston
Copy link
Member

Eloston commented Dec 15, 2016

The commit message is fine. Good implementation of the changes. We should probably disable downloading syzygy for now by commenting out the lines in extra_deps.ini so there won't be extra downloading.

Also what is the problem with the original code for patching? What kind of errors (or any output) do you get? The GNU patch utility I instructed users to use in BUILDING works on Windows 10.

@nopjmp
Copy link
Contributor Author

nopjmp commented Dec 15, 2016 via email

@Eloston
Copy link
Member

Eloston commented Dec 15, 2016

The patch function outputs patching but it doesn't patch anything. I can look further into it.

That's really weird. I have no idea what can be causing that if the patch utility hasn't changed and buildlib's invocation of it hasn't changed. Maybe it's some weird Windows silent failure?

@nopjmp
Copy link
Contributor Author

nopjmp commented Dec 16, 2016

I think it might be due to how python is interacting with the shell, but stdin might have been acting strangely in my setup. I'm not using ActivePython, but the official release. It might work if I updated python 3 I just noticed I'm on a REALLY old version.

C:\Users\nopjmp>C:\Python34\python.exe
Python 3.4.3 (v3.4.3:9b73f1c3e601, Feb 24 2015, 22:44:40) [MSC v.1600 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>>

@Eloston
Copy link
Member

Eloston commented Dec 16, 2016

@nopjmp Ah yeah Python 3.5 changed some stuff with the subprocess module. You should upgrade since I haven't tested the behavior with Python 3.4 on Windows (someone has on Debian Jessie though, but it's different from Windows).

@nopjmp
Copy link
Contributor Author

nopjmp commented Dec 16, 2016

@Eloston 64bit builds, but I need to fix GN bootstrap.py to compile for 64bit. Otherwise, it works just fine. I'll try to clean it up today if I have time; otherwise, it will be done tomorrow. I'll test Python3.5 also.

@Eloston
Copy link
Member

Eloston commented Dec 16, 2016

@nopjmp Alright sounds good. Glad to hear that we're almost there.

@nopjmp
Copy link
Contributor Author

nopjmp commented Dec 16, 2016

Windows 64bit is fixed and I reverted the hacked up patch function.

@Eloston
Copy link
Member

Eloston commented Dec 16, 2016

Perfect. Even all of the patches and code are consistent in style with everything else. Thanks for the great work!

@Eloston Eloston merged commit 3fb7d34 into ungoogled-software:master Dec 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants