Closed
Bug 974900
Opened 11 years ago
Closed 10 years ago
The Skia build is no longer unified as of bug 910754
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: bjacob, Assigned: bjacob)
References
Details
Attachments
(4 files, 2 obsolete files)
9.07 KB,
patch
|
gw280
:
review+
|
Details | Diff | Splinter Review |
1.87 KB,
patch
|
gw280
:
review+
|
Details | Diff | Splinter Review |
6.01 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
10.42 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
Seems like the UNIFIED_SOURCES moz.build was overwritten.
Comment 1•11 years ago
|
||
Yeah this was fallout from the new script we have to generate the moz.build. When I started on it I just didn't want to have to deal with problems due to unified build. We should try to fix it now, though.
Assignee | ||
Comment 2•11 years ago
|
||
If we agree that build times matter at all, then this is really worth fixing!
Comment 3•11 years ago
|
||
When I try to run this script, I get:
$ ./gyp_mozbuild
cp: cannot create regular file `trunk/third_party/externals/gyp/pylib/gyp/generator': No such file or directory
sh: 1: ./gyp_skia: not found
Failed to generate sources for linux
sh: 1: ./gyp_skia: not found
Failed to generate sources for mac
sh: 1: ./gyp_skia: not found
Failed to generate sources for android
sh: 1: ./gyp_skia: not found
Failed to generate sources for win
Traceback (most recent call last):
File "./generate_mozbuild.py", line 306, in <module>
main()
File "./generate_mozbuild.py", line 302, in main
write_mozbuild(includes, separated_sources)
File "./generate_mozbuild.py", line 271, in write_mozbuild
write_list(f, 'SOURCES', sources['mac'], 4)
KeyError: 'mac'
It seems like the gyp_skia script was not checked into the tree.
snorp, can you please take a look at this?
Flags: needinfo?(snorp)
Comment 4•11 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo? me!) (slow responsiveness, emailapocalypse) from comment #3)
> When I try to run this script, I get:
>
> $ ./gyp_mozbuild
> cp: cannot create regular file
> `trunk/third_party/externals/gyp/pylib/gyp/generator': No such file or
> directory
> sh: 1: ./gyp_skia: not found
> Failed to generate sources for linux
> sh: 1: ./gyp_skia: not found
> Failed to generate sources for mac
> sh: 1: ./gyp_skia: not found
> Failed to generate sources for android
> sh: 1: ./gyp_skia: not found
> Failed to generate sources for win
> Traceback (most recent call last):
> File "./generate_mozbuild.py", line 306, in <module>
> main()
> File "./generate_mozbuild.py", line 302, in main
> write_mozbuild(includes, separated_sources)
> File "./generate_mozbuild.py", line 271, in write_mozbuild
> write_list(f, 'SOURCES', sources['mac'], 4)
> KeyError: 'mac'
>
> It seems like the gyp_skia script was not checked into the tree.
>
> snorp, can you please take a look at this?
You need to have a full Skia tree under trunk, not just the parts we use, in order to run the gyp_mozbuild script. It should be possible to just have a full tree in m-c now, I think, without too much trouble. George is there a reason we don't do that?
Flags: needinfo?(snorp) → needinfo?(gwright)
Comment 5•11 years ago
|
||
Seems like it would be a huge waste to check in the entire Skia tree including buildsystem into mozilla-central. That's why I never did it. If we think it's worth it, then we can do it?
Flags: needinfo?(gwright)
Comment 6•11 years ago
|
||
(In reply to George Wright (:gw280) from comment #5)
> Seems like it would be a huge waste to check in the entire Skia tree
> including buildsystem into mozilla-central. That's why I never did it. If we
> think it's worth it, then we can do it?
I see some value. You can easily make a standalone build of Skia that way and run their unit tests, etc. This can be useful in order to nail down a bug. Also it lets people modify the moz.build, as in this case.
Comment 7•11 years ago
|
||
OK. Let's import the entire Skia tree with the next update of Skia (slated for the week of March 18th which I believe is when the next train leaves).
Comment 8•11 years ago
|
||
Detailed steps to run the script for now:
- Clone skia from https://code.google.com/p/skia/source/checkout
- Ensure you grab the same revision as is listed in README_MOZILLA in gfx/skia
- Copy that entire source tree to gfx/skia/trunk/
- Run gyp_mozbuild
- Profit
Comment 9•11 years ago
|
||
Sorry if this is a stupid question, but here's what I get following those instructions:
$ ./gyp_mozbuild
cp: trunk/third_party/externals/gyp/pylib/gyp/generator: No such file or directory
Traceback (most recent call last):
File "./gyp_skia", line 29, in <module>
import gyp
ImportError: No module named gyp
Failed to generate sources for linux
Traceback (most recent call last):
File "./gyp_skia", line 29, in <module>
import gyp
ImportError: No module named gyp
Failed to generate sources for mac
Traceback (most recent call last):
File "./gyp_skia", line 29, in <module>
import gyp
ImportError: No module named gyp
Failed to generate sources for android
Traceback (most recent call last):
File "./gyp_skia", line 29, in <module>
import gyp
ImportError: No module named gyp
Failed to generate sources for win
Traceback (most recent call last):
File "./generate_mozbuild.py", line 303, in <module>
main()
File "./generate_mozbuild.py", line 299, in main
write_mozbuild(includes, separated_sources)
File "./generate_mozbuild.py", line 268, in write_mozbuild
write_list(f, 'SOURCES', sources['mac'], 4)
KeyError: 'mac'
The first line still suggests there is a missing file somewhere. Also, where do I get the gyp module for python? I tried cloning http://gyp.googlecode.com/svn/trunk/ but that repo doesn't have any insturctions on how to install this module.
Flags: needinfo?(gwright)
Comment 10•11 years ago
|
||
Checking out the Skia codebase should grab all that for you. Did you just do a straight up git clone or did you use gclient? I think the latter is necessary to grab the externals.
https://sites.google.com/site/skiadocs/developer-documentation/contributing-code/downloading
Flags: needinfo?(gwright)
Comment 11•11 years ago
|
||
(In reply to comment #10)
> Checking out the Skia codebase should grab all that for you. Did you just do a
> straight up git clone or did you use gclient? I think the latter is necessary
> to grab the externals.
>
> https://sites.google.com/site/skiadocs/developer-documentation/contributing-code/downloading
I see, I just cloned skia directly because you said so in comment 8. ;-)
Comment 12•11 years ago
|
||
Just to be sure - we want to automate the whole "update from upstream" Skia process and have it be done daily, without any human intervention. If unified sources either help or hinder that, let's make sure we understand.
Comment 13•11 years ago
|
||
This unifies most of Skia. Most notably, it doesn't yet unify the sources for the gpu and pathops stuff, because there are a lot of build failures there. My opinion is that we should land this then create a new bug to track making gpu/pathops unifiable. This will require coordinating with upstream as the source will need to be modified.
Attachment #8389698 -
Flags: review?(ehsan)
Comment 14•11 years ago
|
||
Comment 15•11 years ago
|
||
Comment on attachment 8389698 [details] [diff] [review]
0001-Unified-sources-for-Skia.patch
Review of attachment 8389698 [details] [diff] [review]:
-----------------------------------------------------------------
Looks great, thanks George!
Attachment #8389698 -
Flags: review?(ehsan) → review+
Comment 16•11 years ago
|
||
Updated try run: https://tbpl.mozilla.org/?tree=Try&rev=fac3e12d81fc
Assignee | ||
Comment 17•11 years ago
|
||
Any news here?
Assignee | ||
Comment 18•11 years ago
|
||
Attachment #8435221 -
Flags: review?(ehsan)
Comment 19•11 years ago
|
||
Comment on attachment 8435221 [details] [diff] [review]
Re-unify the skia build manually
Try run?
Assignee | ||
Comment 20•11 years ago
|
||
Comment 21•11 years ago
|
||
Comment on attachment 8435221 [details] [diff] [review]
Re-unify the skia build manually
Review of attachment 8435221 [details] [diff] [review]:
-----------------------------------------------------------------
Note that I don't know whether we want to upstream these changes, or whether we want to update the string, etc. But as far as what goes into Gecko is concerned, this is good when it builds everywhere!
Attachment #8435221 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 22•11 years ago
|
||
Attachment #8435991 -
Flags: review?(gwright)
Assignee | ||
Comment 23•11 years ago
|
||
Attachment #8435992 -
Flags: review?(gwright)
Assignee | ||
Comment 24•11 years ago
|
||
Attachment #8435221 -
Attachment is obsolete: true
Attachment #8435993 -
Flags: review+
Updated•11 years ago
|
Attachment #8435991 -
Flags: review?(gwright) → review+
Updated•11 years ago
|
Attachment #8435992 -
Flags: review?(gwright) → review+
Assignee | ||
Comment 25•11 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7b5f9225d255
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f0d95520c476
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/123b94215b3e
Assignee: nobody → bjacob
Assignee | ||
Comment 26•11 years ago
|
||
Green try run: https://tbpl.mozilla.org/?tree=Try&rev=108d53f92619
Comment 27•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7b5f9225d255
https://hg.mozilla.org/mozilla-central/rev/f0d95520c476
https://hg.mozilla.org/mozilla-central/rev/123b94215b3e
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Comment 28•10 years ago
|
||
Benoit broke the Skia buildgenerator :(
Let's fix this properly.
Attachment #8389698 -
Attachment is obsolete: true
Attachment #8446338 -
Flags: review?(snorp)
Updated•10 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 29•10 years ago
|
||
Comment 30•10 years ago
|
||
Comment on attachment 8446338 [details] [diff] [review]
0001-Unified-sources-for-Skia.patch
Review of attachment 8446338 [details] [diff] [review]:
-----------------------------------------------------------------
nice
Attachment #8446338 -
Flags: review?(snorp) → review+
Comment 31•10 years ago
|
||
Updated try run: https://tbpl.mozilla.org/?tree=Try&rev=a05763f854bd
Assignee | ||
Comment 32•10 years ago
|
||
\o/
Thanks for this! Skia is one of the largest directories with 23 unified files and unification saves about 3 minutes of CPU time to everyone making a clobber build, so this work is really useful.
Comment 33•10 years ago
|
||
Comment 34•10 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•