Switch From .exe Shim To Batch File

[joefitzgerald]

Adding another line like https://github.com/atom/chocolatey/blob/master/chocolatey/tools/chocolateyInstall.ps1#L27 for atom.bat (Install-BinFile "apm" "$dest\Atom\atom.exe") would fix issues described:

• Here: http://discuss.atom.io/t/testing-ci-on-windows-with-appveyor/10690/12?u=joefitzgerald
• And Here: http://discuss.atom.io/t/unable-to-open-file-folder-from-cli-in-windows-8/10693

Removing the atom.exe file from %ALLUSERSPROFILE%\Chocolatey\bin\atom.exe and replacing it with %ALLUSERSPROFILE%\Chocolatey\bin\atom.bat with the following content allows specs to run via apm test and allows atom . or atom c:\windows to work.

@echo off
SET DIR=%~dp0%
cmd /c "%DIR%..\lib\atom.0.115.0\tools\atom\atom.exe %*"
exit /b %ERRORLEVEL%

What's the rationale for the .exe shim vs a .bat?

[kevinsawicki]

Chocolatey auto-generates those shims, nothing in the Atom chocolatey install explicitly creates them.

I believe the shim issues are being fixed in the next chocolatey release.

https://github.com/chocolatey/chocolatey/blob/master/CHANGELOG.md#09826-unreleased

I explored adding Install-BinFile "atom.bat" "$dest\Atom\atom.exe" but the atom.exe still gets generated since chocolatey generates these shims after the Atom installer finishes.

[joefitzgerald]

It's OK if atom.exe is still there - at least for specs, we can still specify a full path to the .bat file. I have confirmed it works if both .bat and .exe files exist in the %ALLUSERSPROFILE%\Chocolatey\bin directory.

[kevinsawicki]

Yeah, it would work for specs but still fail for people trying to run atom path-to-a-file.js

[joefitzgerald]

Let me install the pre-release bits like @ferventcoder (hey!, weird, we're not talking in https://github.com/joefitzgerald/packer-windows) suggested in the issue and see if that works around the issue temporarily.

[joefitzgerald]

So the folder issue is fixed. I think the issue with specs is that the atom.exe shim exits immediately and so apm test --path %ALLUSERSPROFILE%\Chocolatey\bin\atom.exe always succeeds because the shim always exits with code 0. The specs probably run in the background, but you'd never know what happened because you see neither logs nor an exit code.

[ferventcoder]

The shims that open GUI's immediately exit. Almost all of the time people want their shell back once they've opened a GUI app.

[ferventcoder]

I'm guessing we can add an option --shimgen--waitforexit to force waiting for exit
Thoughts?

[joefitzgerald]

@ferventcoder that would be perfect - or alternatively allow that to be an option passed to the shim directly.

[joefitzgerald]

Another alternative would be to generate .bat equivalents for .exe shims - as these already exhibit the correct behavior.

[ferventcoder]

That's what I'm talking about. Try this if you are on the prerelease :
atom --shimgen-log

[ferventcoder]

I don't think that is true, any time the shim/bat was a GUI reference, it immediately gave the shell back. Anytime it's a command line app, it waited for exit. This has been a long time way of working for chocolatey, even with .bats.

[joefitzgerald]

OK - this may be a function of the fact that the .bat is being passed to apm which may be using node to launch the process using child_process.

So yeah, maybe it is better to focus on making the .exe wait.

[kevinsawicki]

@ferventcoder --shimgen--waitforexit sounds useful 👍

[ferventcoder]

chocolatey-archive/chocolatey#509

[ferventcoder]

Added it to the original ticket.

[ferventcoder]

Also pushed alpha2 for you to give this a shot and see if it resolves your issue.

[joefitzgerald]

Issues installing now:

c:\projects\go-plus>choco install atom -f
Chocolatey (v0.9.8.26-alpha2) is installing 'atom' and dependencies. By installing you accept the license for 'atom' and each dependency you are installing.

Atom v0.115.0
Extracting C:\Users\vagrant\AppData\Local\Temp\chocolatey\Atom\AtomInstall.zip to C:\ProgramData\chocolatey\lib\Atom.0.115.0\tools...
Write-Error : Atom did not finish successfully. Boo to the chocolatey gods!
-----------------------
[ERROR] Process has exited, so the requested information is not available.
-----------------------
At C:\ProgramData\chocolatey\chocolateyinstall\helpers\functions\Write-ChocolateyFailure.ps1:30 char:3
+   Write-Error $errorMessage
+   ~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [Write-Error], WriteErrorException
    + FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Write-Error

Write-Error : Package 'Atom v0.115.0' did not install successfully: Process has exited, so the requested information is not available.
At C:\ProgramData\chocolatey\chocolateyinstall\functions\Chocolatey-NuGet.ps1:90 char:17
+                 Write-Error "Package `'$installedPackageName v$installedPackageV ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [Write-Error], WriteErrorException
    + FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Write-Error

Finished installing 'atom' and dependencies - if errors not shown in console, none detected. Check log for errors if unsure.

[joefitzgerald]

@ferventcoder more detail: https://gist.github.com/joefitzgerald/67eeac2aeef293981a9c

Contents of failure.log:

Atom did not finish successfully. Boo to the chocolatey gods!
-----------------------
[ERROR] Process has exited, so the requested information is not available.
-----------------------

[ferventcoder]

Could be running from a non-administrative process, with the new install location it requires administrative shells.

[ferventcoder]

This could be the posh v3+ from cmd.exe issue that we just fixed - chocolatey-archive/chocolatey#516

[joefitzgerald]

Running as admin:

Windows 2012 R2, built via Packer-Windows, with which you are familiar...

[ferventcoder]

Install the new release and let's see if it fixes the issue. Let's also make sure you don't have anything holding a lock on the target folder (Lockhunter is a good tool to install for this).

I didn't have errors running this so I pushed the new release.

[joefitzgerald]

Rebooted after install to v0.9.8.26: https://gist.github.com/joefitzgerald/cc038da8ecf194e211b1

Still no dice. A rather cryptic error.

[ferventcoder]

Check the file C:\ProgramData\chocolatey\lib\Atom.0.115.0\AtomInstall.zip.txt and see what the contents are. Whatever the next file that should have been profiled may have caused the issue.

[joefitzgerald]

It's not there. Also, chocolatey moves that directory to C:\ProgramData\chocolatey\lib-bad\Atom.0.115.0 (note: lib-bad).

[ferventcoder]

Not there in the lib-bad directory either?

[joefitzgerald]

In neither location.

[ferventcoder]

Yes, this is very cryptic.

[joefitzgerald]

C:\ProgramData\chocolatey\lib does not contain an Atom.0.115.0 directory. When I do an install I see it created there and then I assume it is moved.

[joefitzgerald]

I wonder if this is an issue with 7-zip?

[joefitzgerald]

c:\ProgramData\chocolatey\lib\Atom.0.115.0>7z x -o "C:\ProgramData\chocolatey\lib\Atom.0.115.0\tools" -y "C:\Users\vagrant\AppData\Local\Temp\chocolatey\Atom\AtomInstall.zip"


Error:
Incorrect command line

Hmmm.....

[joefitzgerald]

Nevermind... removing the space between -o and " fixes it.

[joefitzgerald]

It does look like the files are extracted when you inspect the tools directory (C:\ProgramData\chocolatey\lib-bad\Atom.0.115.0\tools\Atom has the files you would expect); so the issue is not 7-zip; it's something that occurs after it.

[ferventcoder]

Run this for me

powershell
$host.version

[joefitzgerald]

c:\ProgramData\chocolatey>powershell
Windows PowerShell
Copyright (C) 2013 Microsoft Corporation. All rights reserved.

PS c:\ProgramData\chocolatey> $host.version

Major  Minor  Build  Revision
-----  -----  -----  --------
4      0      -1     -1

[joefitzgerald]

c:\ProgramData\chocolatey>where powershell
C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe

[ferventcoder]

I think it's the specific change in wait-process causing issues.

[ferventcoder]

just reproduced it in Posh v4.

[ferventcoder]

Going to yank 0.9.8.26 for now.

[joefitzgerald]

Interestingly, the AppVeyor build is not affected (although I have to update it to use the shim and the new Chocolatey version): https://ci.appveyor.com/project/joefitzgerald/go-plus/build/1.0.36

Perhaps I should update Packer-Windows to install a newer version of PowerShell...

[ferventcoder]

It's definitely the new wait-process. We need to be a little smarter around that, but it's going to need to wait until later today or tomorrow. Family day...

[ferventcoder]

The smoke tests unfortunately didn't catch this.

[ferventcoder]

@joefitzgerald I'm pushing 0.9.8.27-beta1 in a few moments. Can you give me the thumbs up on that? If it works for you, I will release today.

[ferventcoder]

I just pushed it. Please let me know if you can check that out today. If not, no worries, I'm pretty confident that the issue has been resolved.

[joefitzgerald]

Will get to this in a few hours, and I'll report back soon after. Thanks!!

[joefitzgerald]

@ferventcoder OK the Atom install succeeded, but the extra flag doesn't help because it is not passed on via apm when atom is being launched. I suspect the only real solution here is using the batch file approach - either targeting the shim or the versioned path to atom.exe.

[joefitzgerald]

Having a batch file at c:\ProgramData\chocolatey\bin\atom.bat with the following content works:

@echo off
SET DIR=%~dp0%
cmd /c "%DIR%..\lib\atom.0.115.0\tools\atom\atom.exe %*"
exit /b %ERRORLEVEL%

Result:

c:\projects\go-plus>apm test --path c:\ProgramData\chocolatey\bin\atom.bat
[1992:0713/145636:ERROR:gpu_info_collector_win.cc(102)] Can't retrieve a valid WinSAT assessment.
[2196:0713/145636:INFO:renderer_main.cc(227)] Renderer process started
warning: removing Breakpad handler out of order
...................................

Finished in 5.485 seconds
35 tests, 182 assertions, 0 failures, 0 skipped


Tests passed

If I change it to target the shim, it still works (tests are run):

@echo off
SET DIR=%~dp0%
cmd /c "%DIR%atom.exe --shimgen--waitforexit %*"
exit /b %ERRORLEVEL%

Result:

c:\projects\go-plus>apm test --path c:\ProgramData\chocolatey\bin\atom.bat
[880:0713/150102:ERROR:gpu_info_collector_win.cc(102)] Can't retrieve a valid WinSAT assessment.
[2504:0713/150103:INFO:renderer_main.cc(227)] Renderer process started
warning: removing Breakpad handler out of order
...................................

Finished in 5.563 seconds
35 tests, 182 assertions, 0 failures, 0 skipped


Tests passed

Removing the --shimgen--waitforexit results in tests not being run:

@echo off
SET DIR=%~dp0%
cmd /c "%DIR%atom.exe %*"
exit /b %ERRORLEVEL%

Result:

c:\projects\go-plus>apm test --path c:\ProgramData\chocolatey\bin\atom.bat
Tests passed

This confirms that the shim is now functioning correctly, but it's still not possible to achieve the outcome we need without writing a batch file. Is it possible to additionally generate a batch file to target the shim or have the shim default to --shimgen--waitforexit?

[kevinsawicki]

@joefitzgerald Thanks for digging into this. I'm fine adding custom chocolatey support to apm such that apm test just works on Windows if you have Atom installed using chocolatey using something like the batch file you have there.

Also, it would be great to add (and blog about) an appveyor setup on https://github.com/atom/ci once it is good to go.

[joefitzgerald]

@kevinsawicki yep, that might be appropriate. At first glance, this would require:

• Always add the --shimgen-waitforexit argument to atom.exe when calling it from apm
• Detect atom.exe by looking for it at: %CHOCOLATEYINSTALL%\bin\atom.exe and %ALLUSERSPROFILE%\chocolatey\bin\atom.exe
• Ensure that when a custom path is passed to apm that the --shimgen-waitforexit argument is still added

[ferventcoder]

@joefitzgerald thanks for digging in. I'd suggest updating the chocolatey package to additionally create the atom.bat file, as choco works in older versions, the GUI apps (having the atom.exe.gui file) generate a batch file that immediately returns the console. So definitely recommend adding the additional batch file. 👍

[joefitzgerald]

Also, @kevinsawicki I'd be happy to make a PR against https://github.com/atom/ci and / or help you with an article on AppVeyor if you'd like.

[joefitzgerald]

@kevinsawicki for your reference:

• PR to fix apm's behavior: Ensure apm test works correctly for chocolatey-installed Atom on Windows apm#149
• PR to add AppVeyor to the CI repo: Add AppVeyor Content ci#3