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

Broke compatibility with PlatformIO #47350

Closed
dnk1000 opened this issue Apr 6, 2018 · 23 comments
Closed

Broke compatibility with PlatformIO #47350

dnk1000 opened this issue Apr 6, 2018 · 23 comments
Assignees
Labels
candidate Issue identified as probable candidate for fixing in the next release important Issue identified as high-priority tasks Task system issues verified Verification succeeded

Comments

@dnk1000
Copy link

dnk1000 commented Apr 6, 2018

Issue Type: Bug

Try and compile a program for ESP-IDF using Platform IO

VS Code version: Code 1.22.1 (950b8b0, 2018-04-06T02:26:57.615Z)
OS version: Windows_NT x64 6.1.7601

System Info
Item Value
CPUs Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz (4 x 2808)
Memory (System) 7.86GB (3.36GB free)
Process Argv C:\Program Files\Microsoft VS Code\Code.exe
Screen Reader no
VM 0%
Extensions (7)
Extension Author (truncated) Version
code-gnu-global aus 0.2.2
vscode-github Kni 0.27.1
python ms- 2018.3.1
cpptools ms- 0.16.1
platformio-ide pla 0.14.0
vscode-icons rob 7.22.0
debug web 0.22.0

(3 theme extensions excluded)

@gschintgen
Copy link

Same here. The auto-update of Visual Studio Code broke my platformio setup. There's also a report on platformio's tracker with some more details:
platformio/platformio-vscode-ide#97
This is a serious regression, I just don't know if it's a bug in vscode or platformio.

@roblourens
Copy link
Member

  • Do you have a .vscode/tasks.json file? Can you post it?
  • In the upstream issue it shows the terminal writing a path to platformio.exe. Is the path correct?

@roblourens roblourens added the info-needed Issue requires more information from poster label Apr 6, 2018
@gschintgen
Copy link

There's no tasks.json file in my project's .vscode folder. (Is that to be expected?)
No, the path to platformio.exe is not correct and AFAICT it should not have been added to the command. I.e. simply calling "platformio run" (without specifying the path to the .exe) in the project's source folder works just fine if I open the integrated PlatformIO Terminal.
I can't remember if the previous version of vscode (probably 1.21) also added a (correct) path to the .exe or if it simply relied on the .exe being found through the PATH variable.

@roblourens
Copy link
Member

That's ok, it sounds like the extension is contributing a task and either vscode is interpreting it incorrectly or the task is not formed correctly. @dbaeumer or the extension maintainer can investigate more.

@ivankravets
Copy link

ivankravets commented Apr 6, 2018

Wow! 😱

It seems like an URGENT bug in VSCode. Now, VSCode append WORKSPACE_ROOT for the each task's executable? Why?

We don't specify a FULL path for an executable program because we don't know really where platformio.exe will be installed. Someone can install it in the global path, and it should work.
That is why we rely on the system environment variable PATH.

I don't know what should we do now. In theory, we can check all paths in system environment PATH and resolve the full path... However, why should we do that? A system handles that automatically, not only Windows.

See https://github.com/platformio/platformio-vscode-ide/blob/develop/src/tasks.js#L261

@dbaeumer does it mean that the fix will be published in 2 weeks? We have ~170K active developers of VSCode + PlatformIO. We will receive very negative feedback :(

Should we work on the quick fix on our side?

Thanks in advance!

P.S: I appreciate your work on the Task API! Thanks! I saw it in a changelog. That is really what do we need to make a significant improvement for PIO + VSCode.

P.S.S: Please take a look at the screenshot below. VSCode appends WORKSAPCE_ROOT before executable file. As result, there is no platformio.exe in WORKSAPCE_ROOT.

screen shot 2018-04-06 at 15 15 01

@ivankravets
Copy link

ivankravets commented Apr 7, 2018

@Tyriar is this bug related to Terminal?

It totally blocked PlatformIO+VSCode on Windows https://community.platformio.org/t/platformio-run-upload-buttons-no-more-working-after-vsc-update-1-22-1/3899

@ivankravets
Copy link

A temporary solution is to roll back to 1.21 using https://code.visualstudio.com/updates/v1_21 and "Download" links.

Also, maybe you will need to disable automatic updates for VSCode.

@Tyriar Tyriar added important Issue identified as high-priority tasks Task system issues and removed info-needed Issue requires more information from poster labels Apr 8, 2018
@Tyriar
Copy link
Member

Tyriar commented Apr 8, 2018

@dbaeumer maybe a candidate for the recovery release? I'm not sure what caused this so I'm assuming it's on the tasks side.

@ivankravets so it's Windows only?

@ivankravets
Copy link

Yes, the only Windows is affected.

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

I am able to reproduce this. Will look into what broke it.

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

Under Windows I wrote my own code to find the executable on the path. This got broken with b1adb21 which fixed #39720

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

I started to debug why this is broken and the problem is that I when VS Code tries to find the executable the C:\Users\dirkb.platformio\penv\Scripts is not on the PATH in my system (e.g. the renderer) but it is in the terminal. Need to understand why this is the case.

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

Understand it now. The reason for the breakage is that Platform IO brings its own PATH environment and findExecutable doesn't consider this. It only takes the PATH from the current renderer process.

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

The interesting questions is why that worked in the first place since under Windows the OS doesn't try to find an executable on the PATH when launching a process. The PATH must always be absolute.

@Tyriar did the terminal or some of the underlying libraries get code to find the executable on the PATH under Windows. This would explain why it worked before. If this is the case then I could simply remove my code :-)

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

OK. I found the code: https://github.com/Tyriar/node-pty/blob/master/src/win/path_util.cc#L31

However that only works if the extension is already provided. So I need to keep my code since simply using platformio would have been enough. The task system then looks for platformio.com and platformio.exe

@dbaeumer dbaeumer added the candidate Issue identified as probable candidate for fixing in the next release label Apr 9, 2018
@dbaeumer dbaeumer added this to the March Recovery 2018 milestone Apr 9, 2018
@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

To verify:

The build task should succeed.

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

Reopening the issue to discussion in bug standup.

@dbaeumer dbaeumer reopened this Apr 9, 2018
@Tyriar
Copy link
Member

Tyriar commented Apr 9, 2018

OK. I found the code

Yes that's it, there's an open issue to improve this: microsoft/node-pty#112

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

@Tyriar thanks. I wrote which in TS if that would help for you. It is here: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts#L691

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

We decided to include the fix in the fix in the next recovery build.

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

Merged in release/1.22

@dbaeumer dbaeumer closed this as completed Apr 9, 2018
@Tyriar
Copy link
Member

Tyriar commented Apr 9, 2018

@dbaeumer that does indeed help 👍 is this something we can push to node-pty and rely on it there?

@dbaeumer
Copy link
Member

dbaeumer commented Apr 9, 2018

@Tyriar feel free to take the code and push it to node-pty. If it is there I will drop mine.

@weinand weinand added the verified Verification succeeded label Apr 11, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators May 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
candidate Issue identified as probable candidate for fixing in the next release important Issue identified as high-priority tasks Task system issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

7 participants