Add a better python checker

[avdg]

No description provided.

[avdg]

Will still add some better error messages... more commits comming soon

[avdg]

I guess I'm done now...

[winstliu]

Note-it should be "install", not "installed".

[avdg]

Done

[jrsconfitto]

i suggest moving the which file either to the utils or the vendor folders instead of creating a new folder for it. The maintainers might have other ideas of what that should go, though.

Thanks for doing this 😃!

[jrsconfitto]

i'm not sure that following this instruction will work correctly. Maybe it could echo some of the other instructions instead.

[avdg]

Yeah, that was the original message.

[avdg]

Pls take a look at the path... I guess its already in a new vendor directory.

I didn't feel like putting it in the utils directory because the file isn't executable (compared to the others)

avdg@bb83fa5

[jrsconfitto]

Should we remove this? I'm guessing it will always fail.

[avdg]

We could, though I prefer to keep the file as it was originally. I don't want to end up in a situation where we don't know what was patches and what was not.

[jrsconfitto]

Hm, i disagree. Git is great for keeping track of what was patched!

[avdg]

Can't disagree. I'll just comment it out.

[jrsconfitto]

ah, i was thinking about using the vendor folder that already exists at the root... but i think the atom people would have a better idea of where to put that.

[avdg]

That can always be done later.

[probablycorey]

This is a good start, but I think we can simplify it bit. I'd like to avoid vendoring the which module if possible. We have been actively trying to remove all vendor dependencies because it makes it hard keep things up-to-date.

Also, with this change the majority of the bootstrap file is now devoted to testing for python. I think it would be better to put these in another file that exports a method named something like 'verifyRequirements`.

We really only need a subset of the checkPythonVersion method and the guessPython method.

[avdg]

This is a good start, but I think we can simplify it bit. I'd like to avoid vendoring the which module
if possible. We have been actively trying to remove all vendor dependencies because it makes it
hard keep things up-to-date.

I understand that maintaining vendor files can be a pain, even with a fairly stable library it still steals some focus which we could use elsewhere.

The best alternative I know so far is to move the which library as npm package, but that would require the checks to be executed after installing the npm packages. Though I welcome other suggestions.

Also, with this change the majority of the bootstrap file is now devoted to testing for python. I think it would be better to put these in another file that exports a method named something like 'verifyRequirements`.

Just an idea... lets move the python checker in the script/utils folder and let it be a script on its own...

Edit: maybe its not a good idea to make it its own script. Npm dependencies... (if we do the thing above)

We really only need a subset of the checkPythonVersion method and the guessPython method.

I'm pretty sure we'll end up with a total modified version of nody-gyp's original source code. I'll sure remove the commented log function.

Edit: also, which part of the code do you want to keep?

[avdg]

Squashing some small commits. The original could be found here https://github.com/avdg/atom/compare/pythonCheckOld, but anyway, lets focus on getting stuff done instead of caring about the small stuff or making it bling bling.

My main focus was to fix the current error caused by the python checkerwhich was too strict on windows. Though this pr will also check python under osx and linux and add a vendor file, which luckily seems stable though it still is a vendor file (with ofcourse requiring maintenance).

The discussion can go on, but I guess we are better off with the current code than what is available upstream.

[probablycorey]

@avdg what do you think about #2457.

The main goal is to error when users don't have Python27 installed, but allow people with Python27 installed in non-default locations a way to get around the error. My hope is that this will fix a majority of the windows build issues without having to match node-gyp's dependency code.

[avdg]

Saw it, and commented it.

[avdg]

Anyway... to match requirements I will eventually move some code to verify-requirements.

I hope to get it up tomorrow

[avdg]

Also, with this change the majority of the bootstrap file is now devoted to testing for python. I think it would be better to put these in another file that exports a method named something like 'verifyRequirements`.

@probablycorey The patch made me make sense of this. Also relooked the utils/ folder and found other js files. So its not a exectuble-only folder. Thanks!

[avdg]

Please test.

[probablycorey]

Thanks for your help on this @avdg. I'm going to close this and move the conversation over to #2457