-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Globally-installed linter used instead of local copy #153
Comments
@jonoward I am unable to reproduce this on my cygwin system. I tried with git's core.autocrlf option unset, set to true, and set to false. With each setting, I did a fresh clone of the sources and then ran ./build/lint.sh, and all three times I got 0 errors. If you can still reproduce this, can you please send me a tarball of your working tree for comparison? |
I've got latest master with no modifications and I can still repro. Here is my working tree + linter output: https://dl.dropboxusercontent.com/u/42310035/shaka-master.tar.gz Perhaps it's something environmental with my Cygwin setup, could I be on an older version of Closure? Or is everything contained within the working tree? |
Basically everything but bash, python, and java is contained within the working tree, including compiler & linter. I'll try with your tarball and see if I can repro. If so, I'll dissect the differences between your working tree and my own. Otherwise, we can try to debug and find the environmental difference. Thanks! |
Looks like a newline problem. Your whole repo has a different newline style than mine. I'm getting 8MB from "git diff". :-) What tools are you using? Are you using the cygwin version of git? There's a .gitattributes file to force unix-style newlines, but in your case, it appears not to have worked. I'm wondering if you have a checkout that predates that file or if it was otherwise ignored somehow. If you do a fresh clone of the repository, does this linter problem go away? |
Actually, I'm getting no linter errors even with the DOS newlines. Since the linter is written in python, perhaps its your python implementation? Are you using the cygwin version of python? |
Yup I'm using the Cygwin version of Python, version 2.7.5-3. Which are you using? |
Sorry for the delay. I missed this in my inbox. I'm running cygwin python 2.7.10. |
I think I've figured out what has happened. It seems I had a globally installed package for closure_linter in C:\cygwin64\lib\python2.7\site-packages which was being used in place of the package within the working tree. I'm not sure what installed this package, but it appeared to be an older version than the one in the working tree. I uninstalled all python related cygwin packages, deleted C:\cygwin64\lib\python2.7\ and reinstalled everything, and now the linter is reporting zero errors as expected on latest master. So seems to be environmental. Cheers |
Ah, I see. Do we need to change the way we import the package to make sure the local copy is used? |
Never mind, I think the answer is yes. In gjslint:
Those should probably be prepended instead. |
Yup good point, prepending to the path would have probably resolved it... |
Reproduced on Linux by globally installing closure_linter and then breaking it:
|
Okay, this is now fixed. Sorry it took me so long to recognize the real problem, and thanks both for the report and your help debugging. |
It appears that running the linter on Cygwin produces more errors (false positives?) than on Linux. For example, see #144.
The text was updated successfully, but these errors were encountered: