[WIP] Support for Windows ( bfirsh/whalebrew#29 ) #34

3846masa · 2017-01-31T18:48:58Z

Whalebrew on Windows 🐋 🎉 🐳 ( #29 )

I checked on cmd and MSYS2 bash with Docker for Windows.

PR for Windows

syscall -> exec (3846masa@`8a6d783`)

To change syscall.Exec to exec.Command because syscall.Exec causes Error on Windows.

Fix checking args

~~First argument checking hack is not working on Windows. I changed it.~~

Add batch file support (3846masa@`ffbdd15`)

When whalebrew install on Windows, generate batch file.
~~Batch file passes shell file and args to whalebrew.~~

Fix default WHALEBREW_INSTALL_PATH (3846masa@`74db526`)

I have no idea where is good for install_path on Windows.
Temporally, WHALEBREW_INSTALL_PATH is C:\whalebrew if on Windows.

Add install script for Windows (3846masa@`94a625d`)

Install script will download whalebrew.exe and set Path and WHALEBREW_INSTALL_PATH.

`path.Join` -> `filepath.Join` (3846masa@`7453d37`)

c.f. ) http://stackoverflow.com/questions/9371031/how-do-i-create-crossplatform-file-paths-in-go

Rewrite test for Windows (3846masa@`bc12aa0`)

Add `MakePackagePath` function (3846masa@`fe06f35`)

Because package file name is different on Windows and *nix

TODO

CI Test for Windows
Best solution for WHALEBREW_INSTALL_PATH on Windows

This is first time to write golang. Please check changes. 🙏
Thanks.

Foucl · 2017-02-02T09:44:08Z

I can confirm that this works on Win 10 x64, Preview 15019 with latest docker for windows beta release. I used it with cmd, but also have msys2 installed and its binaries in my PATH (don't know if that's relevant).
tiny note: maybe create the install directory if it doesn't exist (whalebrew install currently throws an error if it doesn't).

bfirsh · 2017-02-02T10:58:11Z

main.go

+		// Check if not command exists
+		if _, _, err := cmd.RootCmd.Find(os.Args); err != nil {
+			// Check if file exists
+			if _, err := os.Stat(os.Args[1]); err == nil {


This seems even more flimsy than my hack. ;) e.g. I can envisage lots of stuff unexpectedly not working if they collide with command names.

You could just run whalebrew run [pkg] in the batch script.

bfirsh · 2017-02-02T11:25:07Z

Thanks for this @3846masa! This is a great start.

Some thoughts:

syscall.Exec can't be trivially replaced (exit code won't be returned, signals won't be passed, etc. Probably lots of unknown unknowns here.)
I think we can come up with a neater solution than writing two files. I did the YAML file with shebang method because it works neatly on *nix, but that's clearly not the case on Windows.
I don't run windows, and I'm sure a lot of contributors won't, so we need thorough integration tests and CI to make sure we don't break it. This is the main reason I want to be conservative about including this - it'll slow down development.
Presumably we'll need some different installation instructions for windows, including setting up paths.

mapitman · 2017-02-05T07:40:20Z

Didn't realize this PR existed and I've been working on my own branch to get whalebrew working on Windows. I found a problem with using exec.Command and hooking up stdin, stdout and stderr. I tested with the whalebrew/lynx package and it doesn't work so well. Packages that don't require any input other than what's passed on the command line seem to work fine. Maybe whalebrew can just write out the full Docker command into the batch files?

mapitman · 2017-02-05T08:14:00Z

Even running the whalebrew/lynx docker container directly from the commandline doesn't seem to work very well on Windows. It's not as bad as when using exec.Command from inside whalebrew, but still not very usable.

mapitman · 2017-02-05T08:32:48Z

More info: when you pass a commandline argument to the lynx command, it hangs and I thought it was only on windows, but it is happening on macOS as well:

lynx http://google.com

So, I don't think there is a problem using command.Exec on Windows. I figured out a way to write out the batch files with YAML embedded too.

3846masa · 2017-02-05T12:31:50Z

Thanks for comments. I changed code.

Use exec.Command on only Windows
- Fix to return status code of exec.Command
Batch file includes YAML (This is valid as YAML and batch file)
- This solution generates only one file
```
:: |
  @( whalebrew run %~f0 %* ) & exit /b errorlevel
image: whalebrew/whalesay
```
Revert args hack
- I agree @bfirsh opinion.
- For windows, user just run whalebrew run [pkg] via batch file

3846masa · 2017-02-05T13:10:38Z

I wrote TODO in first comment and changed PR title.

3846masa · 2017-02-13T14:25:53Z

I changed.

Merge v0.0.4 and Fix for Windows
path.Join -> filepath.Join
- 3846masa/whalebrew@7453d37
Rewrite test
- See results via Appveyor and Travis CI.
- 3846masa/whalebrew@bc12aa0
- Add MakePackagePath function
  - Because package file name is different on Windows and *nix
  - 3846masa/whalebrew@fe06f35
- Add appveyor.yml
  - Appveyor is CI for Windows
  - 3846masa/whalebrew@a0e486c
- Fix .travis.yml for forked project
  - 3846masa/whalebrew@21bcf14

3846masa · 2017-09-08T01:59:15Z

Bump to 0.1.0.
Also add install script for Windows.

bfirsh · 2017-09-08T07:46:27Z

Nice, thank you @3846masa! (and @mapitman!) I will take a proper look at this when I get some time.

Would you be willing to help maintain the Windows support? I don't have a Windows machine so I am worried it will deteriorate unless somebody's keeping an eye on it. The CI will certainly help - thanks for that. :)

Whalebrew seems pretty stable and feature complete now, so I doubt there'll be much involved.

3846masa · 2017-09-08T08:47:03Z

Ok, I'll help maintain for Windows.
I want whalebrew on Windows because I use Windows machine everyday 👍

Squashed commit of the following: commit b677dbc Author: 3846masa <[email protected]> Date: Sat Oct 28 11:35:30 2017 +0900 Fix golang version commit 43bc363 Author: 3846masa <[email protected]> Date: Sat Oct 28 11:12:42 2017 +0900 Fix shebang for Windows commit ed225df Author: 3846masa <[email protected]> Date: Fri Sep 8 10:29:39 2017 +0900 Fix CI commit 94a625d Author: 3846masa <[email protected]> Date: Fri Sep 8 10:26:49 2017 +0900 Fix windows installer commit 0bea8fb Author: 3846masa <[email protected]> Date: Fri Sep 8 10:26:20 2017 +0900 Fix README for Windows installer commit 237a4a7 Author: 3846masa <[email protected]> Date: Thu Sep 7 19:11:19 2017 +0900 Fix build for Windows commit bc508d7 Author: 3846masa <[email protected]> Date: Thu Sep 7 18:59:16 2017 +0900 Fix to set API version (moby/moby#32779) commit 442be2f Author: 3846masa <[email protected]> Date: Thu Sep 7 18:03:01 2017 +0900 Update README for Windows commit e83b2ed Author: 3846masa <[email protected]> Date: Thu Sep 7 18:01:01 2017 +0900 Add installer for Windows commit 776b931 Author: 3846masa <[email protected]> Date: Thu Sep 7 18:00:38 2017 +0900 Build for Windows commit bc01aef Author: 3846masa <[email protected]> Date: Thu Sep 7 17:43:43 2017 +0900 Fix `run.go` for Windows commit 63b9490 Merge: a72237f 0fea7e2 Author: 3846masa <[email protected]> Date: Thu Sep 7 17:43:01 2017 +0900 Merge tag '0.1.0' of https://github.com/bfirsh/whalebrew into support-windows commit a72237f Author: 3846masa <[email protected]> Date: Thu Sep 7 17:26:29 2017 +0900 Fix `ForceInstall` for windows commit de458d9 Merge: 03d00bf e7d0792 Author: 3846masa <[email protected]> Date: Thu Sep 7 17:22:41 2017 +0900 Merge tag '0.0.5' into support-windows commit 03d00bf Author: 3846masa <[email protected]> Date: Tue Feb 14 00:01:52 2017 +0900 Fix batch file's shebang commit 21bcf14 Author: 3846masa <[email protected]> Date: Mon Feb 13 22:19:25 2017 +0900 Fix .travis.yml for forked project commit a0e486c Author: 3846masa <[email protected]> Date: Mon Feb 13 21:59:19 2017 +0900 Add appveyor.yml commit fe06f35 Author: 3846masa <[email protected]> Date: Mon Feb 13 21:34:46 2017 +0900 MakePackagePath commit bc12aa0 Author: 3846masa <[email protected]> Date: Mon Feb 13 21:23:31 2017 +0900 Rewrite test commit 7453d37 Author: 3846masa <[email protected]> Date: Mon Feb 13 20:28:33 2017 +0900 `path.Join` -> `filepath.Join` commit 4ded1a2 Author: 3846masa <[email protected]> Date: Mon Feb 13 16:53:51 2017 +0900 LF -> CRLF in batch file commit 8078165 Author: 3846masa <[email protected]> Date: Mon Feb 13 16:34:08 2017 +0900 Fix edit for Windows commit 8746496 Author: 3846masa <[email protected]> Date: Mon Feb 13 16:15:55 2017 +0900 Fix run for windows commit bb652d4 Merge: 8a6d783 e7e6427 Author: 3846masa <[email protected]> Date: Mon Feb 13 16:08:42 2017 +0900 Merge v0.0.4 Merge remote-tracking branch 'origin/master' into support-windows commit 8a6d783 Author: 3846masa <[email protected]> Date: Sun Feb 5 21:11:59 2017 +0900 Support to run and return status code on Windows commit ce3d298 Author: 3846masa <[email protected]> Date: Sun Feb 5 20:58:21 2017 +0900 Revert "syscall -> exec (for working on windows)" This reverts commit 276a9cd. commit ffbdd15 Author: 3846masa <[email protected]> Date: Sun Feb 5 20:50:08 2017 +0900 Add batch file support (for working on windows) commit cdc5a05 Author: 3846masa <[email protected]> Date: Sun Feb 5 20:34:30 2017 +0900 Revert "Add batch file support (for working on windows)" This reverts commit aba79b0. commit 10e10a0 Author: 3846masa <[email protected]> Date: Fri Feb 3 00:53:23 2017 +0900 Revert "Fix checking args (for working on windows)" This reverts commit 926f422. commit 74db526 Author: 3846masa <[email protected]> Date: Wed Feb 1 03:14:59 2017 +0900 Fix default WHALEBREW_INSTALL_PATH (for working on windows) commit aba79b0 Author: 3846masa <[email protected]> Date: Wed Feb 1 03:06:00 2017 +0900 Add batch file support (for working on windows) commit 926f422 Author: 3846masa <[email protected]> Date: Wed Feb 1 01:45:51 2017 +0900 Fix checking args (for working on windows) commit 276a9cd Author: 3846masa <[email protected]> Date: Wed Feb 1 01:36:56 2017 +0900 syscall -> exec (for working on windows)

This allows to move forward in the direction of windows compatibility. This takes part of changes from #34 and makes a step forward for #29.

asadakbarml · 2021-06-09T20:07:48Z

Is there a timeline for this?

tjamet · 2021-06-23T07:27:25Z

Hi @asadakbarml thanks for the heads up.
There is currently no work being done on windows.
I would be happy though to accept pull requests to do so.

bfirsh reviewed Feb 2, 2017

View reviewed changes

bfirsh mentioned this pull request Feb 3, 2017

Add support for Windows #29

Open

3846masa force-pushed the support-windows-rebase branch from 1d95b0f to a7a4f02 Compare February 5, 2017 12:18

3846masa changed the title ~~Support for Windows ( bfirsh/whalebrew#29 )~~ [WIP] Support for Windows ( bfirsh/whalebrew#29 ) Feb 5, 2017

3846masa force-pushed the support-windows-rebase branch 3 times, most recently from cf83dff to 3107cb8 Compare February 13, 2017 13:21

3846masa force-pushed the support-windows-rebase branch from 3107cb8 to 8ea45ff Compare February 13, 2017 15:03

3846masa force-pushed the support-windows-rebase branch from 8ea45ff to 2b5145d Compare September 8, 2017 01:51

3846masa force-pushed the support-windows-rebase branch 2 times, most recently from aea13b2 to c48c383 Compare October 28, 2017 03:01

3846masa force-pushed the support-windows-rebase branch from c48c383 to f74c60d Compare October 28, 2017 03:05

tjamet added a commit that referenced this pull request Oct 14, 2020

Move editor to a distinct package

b1446a1

This allows to move forward in the direction of windows compatibility. This takes part of changes from #34 and makes a step forward for #29.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Support for Windows ( bfirsh/whalebrew#29 ) #34

[WIP] Support for Windows ( bfirsh/whalebrew#29 ) #34

3846masa commented Jan 31, 2017 •

edited

Loading

Foucl commented Feb 2, 2017

bfirsh Feb 2, 2017

bfirsh commented Feb 2, 2017

mapitman commented Feb 5, 2017

mapitman commented Feb 5, 2017

mapitman commented Feb 5, 2017

3846masa commented Feb 5, 2017 •

edited

Loading

3846masa commented Feb 5, 2017 •

edited

Loading

3846masa commented Feb 13, 2017 •

edited

Loading

3846masa commented Sep 8, 2017

bfirsh commented Sep 8, 2017 •

edited

Loading

3846masa commented Sep 8, 2017

asadakbarml commented Jun 9, 2021

tjamet commented Jun 23, 2021

[WIP] Support for Windows ( bfirsh/whalebrew#29 ) #34

Are you sure you want to change the base?

[WIP] Support for Windows ( bfirsh/whalebrew#29 ) #34

Conversation

3846masa commented Jan 31, 2017 • edited Loading

PR for Windows

syscall -> exec (3846masa@8a6d783)

Fix checking args

Add batch file support (3846masa@ffbdd15)

Fix default WHALEBREW_INSTALL_PATH (3846masa@74db526)

Add install script for Windows (3846masa@94a625d)

path.Join -> filepath.Join (3846masa@7453d37)

Rewrite test for Windows (3846masa@bc12aa0)

Add MakePackagePath function (3846masa@fe06f35)

TODO

Foucl commented Feb 2, 2017

bfirsh Feb 2, 2017

Choose a reason for hiding this comment

bfirsh commented Feb 2, 2017

mapitman commented Feb 5, 2017

mapitman commented Feb 5, 2017

mapitman commented Feb 5, 2017

3846masa commented Feb 5, 2017 • edited Loading

3846masa commented Feb 5, 2017 • edited Loading

3846masa commented Feb 13, 2017 • edited Loading

3846masa commented Sep 8, 2017

bfirsh commented Sep 8, 2017 • edited Loading

3846masa commented Sep 8, 2017

asadakbarml commented Jun 9, 2021

tjamet commented Jun 23, 2021

3846masa commented Jan 31, 2017 •

edited

Loading

syscall -> exec (3846masa@`8a6d783`)

Add batch file support (3846masa@`ffbdd15`)

Fix default WHALEBREW_INSTALL_PATH (3846masa@`74db526`)

Add install script for Windows (3846masa@`94a625d`)

`path.Join` -> `filepath.Join` (3846masa@`7453d37`)

Rewrite test for Windows (3846masa@`bc12aa0`)

Add `MakePackagePath` function (3846masa@`fe06f35`)

3846masa commented Feb 5, 2017 •

edited

Loading

3846masa commented Feb 5, 2017 •

edited

Loading

3846masa commented Feb 13, 2017 •

edited

Loading

bfirsh commented Sep 8, 2017 •

edited

Loading