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

Support building script on Arch Linux #20268

Closed
FallingSnow opened this issue Mar 10, 2018 · 5 comments
Closed

Support building script on Arch Linux #20268

FallingSnow opened this issue Mar 10, 2018 · 5 comments

Comments

@FallingSnow
Copy link

@FallingSnow FallingSnow commented Mar 10, 2018

Arch Linux sets python to python 3 and python2 to python 2.

Script's CMakeLists.txt hard codes the python command.

COMMAND python -B ${bindings_src}/pythonpath.py -I ${bindings_src}/parser -I ${bindings_src}/ply

In order to select the correct python version

find_package( PythonInterp 2 REQUIRED )

should be added to the top of the file and all python commands should be replaced with

${PYTHON_EXECUTABLE}

so the above reads

COMMAND ${PYTHON_EXECUTABLE} -B ${bindings_src}/pythonpath.py -I ${bindings_src}/parser -I ${bindings_src}/ply 

P.S. This is supported by the minimum cmake version 2.6

@kwonoj
Copy link
Contributor

@kwonoj kwonoj commented Mar 12, 2018

It'd be good to support build script on arch ooto. Mind if I shoot small PR for it? @FallingSnow (only if you don't have plan to tackle this)

@FallingSnow
Copy link
Author

@FallingSnow FallingSnow commented Mar 12, 2018

By all means, please. I have too many other obligations to submit a PR otherwise I would have already :)

@kwonoj
Copy link
Contributor

@kwonoj kwonoj commented Mar 12, 2018

@FallingSnow thanks, let me try to stab on this.

@kwonoj
Copy link
Contributor

@kwonoj kwonoj commented Mar 12, 2018

@highfive : assign me

@highfive highfive added the C-assigned label Mar 12, 2018
@highfive
Copy link

@highfive highfive commented Mar 12, 2018

Hey @kwonoj! Thanks for your interest in working on this issue. It's now assigned to you!

@jdm jdm added the A-build label Mar 12, 2018
kwonoj added a commit to kwonoj/servo that referenced this issue Mar 13, 2018
- closes servo#20268
kwonoj added a commit to kwonoj/servo that referenced this issue Mar 13, 2018
- closes servo#20268
bors-servo added a commit that referenced this issue Mar 13, 2018
build(cmake): detect python binary for specified version

<!-- Please describe your changes on the following line: -->
This PR aims to address #20268, by updating cmake definition to lookup python binary to use. Instead of directly asking `python` binary, this PR uses `${PYTHON_EXECUTABLE}` to lookup desired python binary where applicable.

This PR was locally tested on win32 / mac / arch linux via `mach build --dev`, so far local testing shows no build failure regressions, not sure if this is sufficient test.

---
<!-- 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
- [x] These changes fix #20268 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because _it's build script update_

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20290)
<!-- Reviewable:end -->
pandusonu2 added a commit to pandusonu2/servo that referenced this issue Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.