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

Port to Windows #216

Merged
merged 8 commits into from Oct 21, 2020
Merged

Port to Windows #216

merged 8 commits into from Oct 21, 2020

Conversation

dil-spayne
Copy link
Contributor

This change set addresses #94 and ports baur to Windows. The make release command will create a baur.exe file.
Things to note:

  1. A breaking change was introduced to golang.org/x/sys that prevents github.com/docker/docker from compiling. This issue in github.com/docker/docker will not be fixed until v20 of github.com/docker/docker is released. In the meantime I have pinned golang.org/x/sys to the latest version before the breaking change was introduced.
  2. Some of the existing tests used commands specific to Unix. These have either been replaced with equivalent commands available directly on both Unix and Windows or run via cmd /C on Windows.
  3. Windows does not allow files that are in use to be deleted in the same way as Unix. In some tests files and folders had to be released so the test cleanup succeeds.
  4. Expected paths have been updated to use OS specific path separators where required.
  5. CircleCI does not provide Windows containers but does provide Windows VMs
    • The pre-installed version of golang is 1.12.7, this needs to be updated on every build. See software-pre-installed-in-the-windows-image
    • I could not access Unix docker containers from within the Windows job so postgres is installed on the VM via chocolatey and the default "postgres" database is used.
    • The tests are much slower to run on Windows than Unix, the test timeout has been increased to accommodate this.
    • The -race flag has been removed from the Windows go test command as a gcc compiler is not installed. I figured this wasn't a problem as it is used in the existing test job. This could be addressed later if necessary.

@dil-spayne
Copy link
Contributor Author

@fho disabling the builds on my fork and opening a new PR worked. The checks have now run.
We can also assign this PR to #94

@fho fho linked an issue Oct 20, 2020 that may be closed by this pull request
@fho fho added the enhancement New feature or request label Oct 20, 2020
@fho fho self-requested a review October 20, 2020 08:46
it keeps the naming consistent and makes it easier to figure out what the
difference between e.g. the test and test_windows targets are.
Copy link
Collaborator

@fho fho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great, thanks a lot for the PR

"testing"
)

func TestEchoStdout(t *testing.T) {
const echoStr = "hello world!"

res, err := Command("echo", "-n", echoStr).Run()
// Windows returns StrOutput with surrounding quotation marks
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The windows echo command prints the string quoted?
Or all output that is returned from running commands via the stdlib exec package is quoted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have investigated this further and the results are quite interesting.
I created a simple app that runs a few different commands with a couple of different inputs, I then ran this on Windows and Linux.
Here are the results:
Running on Windows
image
Running on Linux
image

Summary:

  • cmd adds quotation marks around the string if it contains spaces
  • PowerShell wraps the output onto a new line at the space if it contains spaces
  • more on Linux outputs headers and footers, Windows doesn't

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so the echo command is probably also a shell builtin command like for bash and the implementation of it in cmd and PowerShell differ.
When commands are invoked without a shell, the output that is returned by the exec package should not be quoted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is my understanding.
Windows doesn't have an echo executable so it must be invoked from a shell. However it does have a more executable and when it is invoked the file contents are not quoted.

@dil-spayne
Copy link
Contributor Author

I noticed that the test results were not being reported in CircleCI. After a bit of investigation I found that the go-test-report.xml file was being generated with Unicode encoding which was causing the problem.
c6bba2f converts the file to UTF-8 encoding and now the test results are being reported correctly.

Before:
image

After:
image

@fho fho merged commit 692dc80 into simplesurance:master Oct 21, 2020
@dil-spayne dil-spayne deleted the port_to_windows branch October 21, 2020 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Windows support
2 participants