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

add support for win32 platform python #70

Merged
merged 1 commit into from Jun 21, 2013

Conversation

Projects
None yet
3 participants
@uchida
Contributor

uchida commented Jun 20, 2013

Installation of emacs-jedi fails in win32 platform.
The virtualenv installs pip to $(ENV)/Scripts when sys.platform == 'win32' instead of $(ENV)/bin.
That is why emacs-jedi make fails in win32 platform, and this pull request fixes this problem.

The path behavior of virtualenv described in path_location function in virtualenv.py, see https://github.com/pypa/virtualenv/blob/develop/virtualenv.py#L924 for more information.

Makefile Outdated
JEDI_DEV_URL = https://github.com/davidhalter/jedi/archive/dev.zip
PYTHON ?= python
CARTON ?= carton
export EMACS ?= emacs
PLATFORM = $(shell $(PYTHON) -c "import sys; print(sys.platform)")
ISPYPY = $(shell $(PYTHON) -c "import sys; print(hasattr(sys, 'pypy_version_info'))")
ifeq ($(PLATFORM) $(ISPYPY), win32 False)

This comment has been minimized.

@tkf

tkf Jun 20, 2013

Owner

Is ifeq supported by BSD Make? We fixed some incompatibility with BSD Make in #11 so I don't want to bring some again. Maybe @goro1080 knows?

This comment has been minimized.

@uchida

uchida Jun 20, 2013

Contributor

This pull request is not supported by BSD Make.
However, maybe $(shell ...) is also GNU extension.
Using gmake on BSD-derivatives may be an alternative way to make.

This comment has been minimized.

@goro1080

goro1080 Jun 21, 2013

Contributor

@uchida is right. ifeq is not work on BSD make.

Instead of ifeq, how about add these definition to Makefile?

BINDIR ?= bin
LIBDIR ?= lib

and call make by make BINDIR=Script LIBDIR=Lib on win32 environment?

This comment has been minimized.

@tkf

tkf Jun 21, 2013

Owner

@uchida @goro1080 Thanks for the clarification.

This comment has been minimized.

@uchida

uchida Jun 21, 2013

Contributor

I tested make on FreeBSD 9.1R.
BSD Make fails $(addprefix ...) and $(shell ...), it takes much more work to handle both BSD and GNU Make.

For example, following works on GNU's and doesn't work on BSD's

PLATFORM = $(shell $(PYTHON) -c "import sys; print(sys.platform)")

works on BSD's and doesn't work on GNU's

PLATFORM != $(PYTHON) -c "import sys; print(sys.platform)"
@tkf

This comment has been minimized.

Owner

tkf commented Jun 20, 2013

Otherwise, it looks good. I am glad someone fixes it for Windows users. Ah, and don't worry about the error in Travis CI.

@uchida

This comment has been minimized.

Contributor

uchida commented Jun 20, 2013

Making a comment, giving PYTHON=\path\to\python.exe will be required to install, but I don't know which python would be better in default.

@tkf

This comment has been minimized.

Owner

tkf commented Jun 21, 2013

@uchida Do you mean Windows user always needs to pass PYTHON? Or can it be on the path so that make automatically picks up?

If Windows user must pass PYTHON, I think passing BINDIR manually is better than using GNU's ifeq syntax.

We can have GNUMakefile and BSDMakefile, but I'd like to avoid if I can... Probably we should go to portable solution such as waf. But it can be done later, not in this PR.

@uchida

This comment has been minimized.

Contributor

uchida commented Jun 21, 2013

@tkf On Windows, users need to pass PYTHON with .exe.
It seems better that passing PYTHON and BINDIR manually as @tkf @goro1080 say.

Makefile Outdated
@@ -69,7 +79,7 @@ print-deps: elpa requirements
@echo "----------------------- Dependencies -----------------------"
$(EMACS) --version
${VIRTUAL_EMACS} -Q -batch -l jedi.el -f jedi:print-jedi-version
ls -d $(ENV)/lib/python*/site-packages/*egg-info
ls -d $(ENV)/$(LIBDIR)/python*/site-packages/*egg-info

This comment has been minimized.

@tkf

tkf Jun 21, 2013

Owner

Let's use $(ENV)/*/python*/... here. This is only used in testing and it is not much important to have an accurate path. This way we can get rid of LIBDIR.

@tkf

This comment has been minimized.

Owner

tkf commented Jun 21, 2013

OK. Then let's go with @goro1080's idea.

@uchida You can revert the commit and force push to not include the first commit. Or you can just open another pull request. Or you can just wait, as I will try to make it compatible for Linux/BSD/Windows sometime later.

@uchida

This comment has been minimized.

Contributor

uchida commented Jun 21, 2013

OK. I took a first one.
Commits were meld into a single one.
Thanks!

tkf added a commit that referenced this pull request Jun 21, 2013

Merge pull request #70 from uchida/win32-support
add support for win32 platform python

@tkf tkf merged commit 44eb7ef into tkf:master Jun 21, 2013

1 check failed

default The Travis CI build failed
Details
@tkf

This comment has been minimized.

Owner

tkf commented Jun 21, 2013

Thank you!

It's not urgent, but if you have some time, please write the instruction for Windows user in the document (doc/source/index.rst).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment