-
Notifications
You must be signed in to change notification settings - Fork 428
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
test convert's handling of win entry points better. Fix it. #1721
Conversation
Sorry, just realized this won't quite work yet. I have to be less lazy and actually compute the file attributes for the bat file, or else conda will probably barf. I'll test that. |
OK, should be working now. I have tested that this really works by building the recipe for the test entry points on OS X. I ran conda-convert in windows, targeting windows output. I installed the package from the converted output (note that dependencies were not installed, because installing paths with conda doesn't install dependencies. =( ) The entry points all work. @gomyhr, I'd feel even better about this if you tested it and made sure that it works for you. Do you have time? |
3ee9249
to
1396737
Compare
1396737
to
eb96270
Compare
@@ -32,7 +32,7 @@ | |||
@echo off | |||
set PYFILE=%~f0 | |||
set PYFILE=%PYFILE:~0,-4%-script.py | |||
"%~f0\\..\\..\\python.exe" "%PYFILE%" %* | |||
"%~dp0\..\python.exe" "%PYFILE%" %* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that the entry point scripts ever worked. This expression was just wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entry points worked fine before, but the new version is more direct (and maybe more robust). The old one would be
C:\path\to\anaconda\Scripts\entrypoint.bat\\..\\..\\python.exe
which works, although somewhat convoluted (and the double slashes are unneccessary). The new one gives
C:\path\to\anaconda\Scripts\..\python.exe
which is more direct.
Windows is actually very forgiving in evaluating paths with .., as this example shows:
C:\Users\gom\AppData\Local\Continuum\Miniconda2>this\path\\does\not\\\exist\thisfileneither.exe\..\..\..\..\..\..\python.exe --version
Python 2.7.12 :: Continuum Analytics, Inc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! Makes sense, I guess. It must just do the parsing of all the .. before anything else. Thanks for teaching me.
d762557
to
40acb03
Compare
40acb03
to
b4b1f7e
Compare
@msaharan, I will have time to test - if not on the weekend, then probably on Monday. |
I have tested, and it works - both the test package described above and another package that I tried. |
I should also mention that I tried to review the commits, but I had to give up as most of them included additional changes, and e.g. what commit eb96270 does, seems unrelated to its commit message "copy data into -script.py/pyc files" |
@gomyhr, thanks for testing. I do sometimes put too much into commits. The one you point out is actually not a case of that, though. Before that commit, what was happening was that the bat file and the py file generated we both just the bat file stub. The actual script was getting lost in the conversion. |
Hi there, thank you for your contribution! This pull request has been automatically locked because it has not had recent activity after being closed. Please open a new issue or pull request if needed. Thanks! |
Fixes #1718
also some flake8 in here.
CC @bryevdv @gomyhr