Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upstylo: Move all binding-generator logic code to a python script. #12212
+1,831
−1,806
Conversation
highfive
commented
Jul 4, 2016
|
|
| @@ -0,0 +1,366 @@ | |||
| #!/usr/bin/env python | |||
This comment has been minimized.
This comment has been minimized.
frewsxcv
Jul 4, 2016
Member
It seems unfortunate this file doesn't get tidy checked.
servo/python/tidy/servo_tidy/tidy.py
Line 74 in 1e2b977
This comment has been minimized.
This comment has been minimized.
emilio
Jul 4, 2016
Author
Member
Whoops... Thanks!
(BTW I meant to mention you above, not @jgraham, who uses to be busy and not too much around servo)
This comment has been minimized.
This comment has been minimized.
emilio
force-pushed the
emilio:stylo-regen-script
branch
2 times, most recently
from
5ff8059
to
01a8ae0
| return ret | ||
|
|
||
|
|
||
| def extend_object(obj, other): |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
emilio
Jul 5, 2016
Author
Member
I don't think so, there's dict.update(other), but that overrides the keys instead of merging them, so we need to write this by hand.
|
r=me with someone looking over the pythonisms. |
|
@bors-servo: r=bholley |
|
|
bors-servo
added a commit
that referenced
this pull request
Jul 6, 2016
stylo: Move all binding-generator logic code to a python script. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors <!-- Either: --> - [x] These changes do not require tests because tooling <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> --- This not only makes us more consistent with the rest of the codebase but also: * Makes us repeat less code like common flags and all that stuff. * Reduces the noise of the build: You only get the output of the commands on failure or when you pass the -v flag. * Makes you able to select a single kind of build or multiple in the same place. I've basically kept the regen.sh script because of the LIBCLANG_PATH checks, but at least from Linux I don't need them anymore. Also, that logic could be moved to the new script. The whole point of this isn't only making it prettier and easier to use, but also allowing me to write more complex logic in the binding generator scripts, that I will probably need to integrate the DOM enum types we need for animations and such easily (can't be just an include, because that pulls in another header with the same name bringing a lot of DOM and IDL churn). For reference, here's a successful regen round with the script: ``` ./regen.py --target all /home/emilio/projects/moz/stylo/gecko/obj-x86_64-pc-linux-gnu/ [BINDGEN] structs::release in "/home/emilio/projects/moz/stylo/gecko/obj-x86_64-pc-linux-gnu/"... OK [RUSTC]... OK [RUSTC_TEST]... OK test result: ok. 168 passed; 0 failed; 0 ignored; 0 measured [BINDGEN] structs::debug in "/home/emilio/projects/moz/stylo/gecko/obj-x86_64-pc-linux-gnu/"... OK [RUSTC]... OK [RUSTC_TEST]... OK test result: ok. 169 passed; 0 failed; 0 ignored; 0 measured [BINDGEN] bindings::None in "/home/emilio/projects/moz/stylo/gecko/obj-x86_64-pc-linux-gnu/"... OK ``` --- r? @bholley Maybe @jgraham and/or @wafflespeanut want to take a look to criticize my (lack of) pythonism ;-) <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12212) <!-- Reviewable:end -->
|
|
|
@bors-servo: retry |
|
|
|
|
This not only makes us more consistent with the rest of the codebase but also: * Makes us repeat less code like common flags and all that stuff. * Reduces the noise of the build: You only get the output of the commands on failure or when you pass the -v flag. * Makes you able to select a single kind of build or multiple in the same place. I've basically kept the regen.sh script because of the LIBCLANG_PATH checks, but at least from Linux I don't need them anymore. Also, that logic could be moved to the new script. The whole point of this isn't only making it prettier and easier to use, but also allowing me to write more complex logic in the binding generator scripts, that I will probably need to integrate the DOM enum types we need for animations and such easily (can't be just an include, because that pulls in another header with the same name bringing a lot of DOM and IDL churn).
…::PlaybackDirection
|
Rebased and addressed review comments of @frewsxcv and @wafflespeanut on IRC. @bors-servo: r=bholley |
|
|
bors-servo
added a commit
that referenced
this pull request
Jul 6, 2016
stylo: Move all binding-generator logic code to a python script. <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors <!-- Either: --> - [x] These changes do not require tests because tooling <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> --- This not only makes us more consistent with the rest of the codebase but also: * Makes us repeat less code like common flags and all that stuff. * Reduces the noise of the build: You only get the output of the commands on failure or when you pass the -v flag. * Makes you able to select a single kind of build or multiple in the same place. I've basically kept the regen.sh script because of the LIBCLANG_PATH checks, but at least from Linux I don't need them anymore. Also, that logic could be moved to the new script. The whole point of this isn't only making it prettier and easier to use, but also allowing me to write more complex logic in the binding generator scripts, that I will probably need to integrate the DOM enum types we need for animations and such easily (can't be just an include, because that pulls in another header with the same name bringing a lot of DOM and IDL churn). For reference, here's a successful regen round with the script: ``` ./regen.py --target all /home/emilio/projects/moz/stylo/gecko/obj-x86_64-pc-linux-gnu/ [BINDGEN] structs::release in "/home/emilio/projects/moz/stylo/gecko/obj-x86_64-pc-linux-gnu/"... OK [RUSTC]... OK [RUSTC_TEST]... OK test result: ok. 168 passed; 0 failed; 0 ignored; 0 measured [BINDGEN] structs::debug in "/home/emilio/projects/moz/stylo/gecko/obj-x86_64-pc-linux-gnu/"... OK [RUSTC]... OK [RUSTC_TEST]... OK test result: ok. 169 passed; 0 failed; 0 ignored; 0 measured [BINDGEN] bindings::None in "/home/emilio/projects/moz/stylo/gecko/obj-x86_64-pc-linux-gnu/"... OK ``` --- r? @bholley Maybe @jgraham and/or @wafflespeanut want to take a look to criticize my (lack of) pythonism ;-) <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12212) <!-- Reviewable:end -->
|
|
|
What gecko-dev commit did you build off? bholley/gecko-dev:stylo isn't working anymore, the attr functions are missing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
emilio commentedJul 4, 2016
•
edited by larsbergstrom
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis not only makes us more consistent with the rest of the codebase but also:
failure or when you pass the -v flag.
place.
I've basically kept the regen.sh script because of the LIBCLANG_PATH checks, but
at least from Linux I don't need them anymore. Also, that logic could be moved
to the new script.
The whole point of this isn't only making it prettier and easier to use, but
also allowing me to write more complex logic in the binding generator scripts,
that I will probably need to integrate the DOM enum types we need for animations
and such easily (can't be just an include, because that pulls in another header
with the same name bringing a lot of DOM and IDL churn).
For reference, here's a successful regen round with the script:
r? @bholley
Maybe @jgraham and/or @wafflespeanut want to take a look to criticize my (lack of) pythonism ;-)
This change is