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

Uses make in windows #325

Merged
merged 3 commits into from
Aug 4, 2021
Merged

Uses make in windows #325

merged 3 commits into from
Aug 4, 2021

Conversation

codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented Aug 3, 2021

This consolidates the build by using the same matrix for Linux, MacOS
and Windows. To do the latter requires installation of CygWin, and to
get GHA to use that requires overriding the shell. One other quirk is
that when you are running tasks via cygwin, it resets the CWD. To avoid
that, this adds the GITHUB_WORKSPACE to .bashrc. Finally, this dodges
various newline related problems and comments why.

Unrelated changes including updating syntax that allows us to
break-apart actions/cache and fixing some lint errors that the build
never caught because lint wasn't run on Windows.

@codefromthecrypt codefromthecrypt force-pushed the try-choco branch 2 times, most recently from 573a234 to 44735a0 Compare August 4, 2021 04:42
@codefromthecrypt codefromthecrypt marked this pull request as ready for review August 4, 2021 04:48
@codefromthecrypt
Copy link
Contributor Author

main goal of this is not just coherency, but allowing us to use this to test using make as a part of #316

@codefromthecrypt
Copy link
Contributor Author

looks like it didn't like caching cygwin..
Uploading Screen Shot 2021-08-04 at 3.23.37 PM.png…

@codefromthecrypt
Copy link
Contributor Author

last didn't error, but it didn't cache either. I'm trying explicit dir for chocolatey

@codefromthecrypt
Copy link
Contributor Author

I'll pull the choco package caching out of scope, as it isn't working, nor is the dominant time in the workflow anyway.

This consolidates the build by using the same matrix for Linux, MacOS
and Windows. To do the latter requires installation of CygWin, and to
get GHA to use that requires overriding the `shell`. One other quirk is
that when you are running tasks via cygwin, it resets the CWD. To avoid
that, this adds the GITHUB_WORKSPACE to `.bashrc`. Finally, this dodges
various newline related problems and comments why.

Unrelated changes including updating syntax that allows us to
break-apart actions/cache and fixing some lint errors that the build
never caught because lint wasn't run on Windows.

Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
@@ -54,7 +54,7 @@ test:
.PHONY: e2e
e2e: $(BIN)
@echo "--- e2e ---"
@E2E_FUNC_E_PATH=$(PWD)/$(BIN) go test -parallel 1 -v -failfast ./e2e
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changing this allowed one less translation problem in cygwin. It is now allowed to be and defaults to relative to the project root.


defaults:
run:
shell: ${{ matrix.shell || 'bash' }}
Copy link
Member

Choose a reason for hiding this comment

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

I didn't know that we can use|| stuff in the GHA expression 😄

@codefromthecrypt codefromthecrypt merged commit 4cc35b0 into master Aug 4, 2021
@codefromthecrypt codefromthecrypt deleted the try-choco branch August 4, 2021 06:57
@codefromthecrypt
Copy link
Contributor Author

Thanks for the look!

@codefromthecrypt
Copy link
Contributor Author

with some adjustments I was able to use the built-in make and save a lot of time and crazy config #327

@codefromthecrypt
Copy link
Contributor Author

#328 simplified further

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants