The Wayback Machine - https://web.archive.org/web/20201020111920/https://github.com/go-sql-driver/mysql/pull/1070
Skip to content

/ mysql

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

Jump to bottom

travis: Add compile check for all supported platforms #1070

Merged
merged 2 commits into from Mar 11, 2020
Merged

travis: Add compile check for all supported platforms #1070

merged 2 commits into from Mar 11, 2020

Conversation

@rayanimesh
Copy link
Contributor

@rayanimesh rayanimesh commented Mar 9, 2020 •
edited

Description

Implements a Travis CI task that checks if the driver compiles on all platforms supported by Go.

Fixes #1050

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file
@shogo82148
shogo82148 reviewed Mar 10, 2020
View changes
Copy link
Contributor

@shogo82148 shogo82148 left a comment

Thanks!
.travis/complie_check.sh Show resolved Hide resolved
@julienschmidt
julienschmidt requested changes Mar 10, 2020
View changes
.travis/complie_check.sh Show resolved Hide resolved
@rayanimesh rayanimesh force-pushed the rayanimesh:travis-compile-check branch from ce66897 to c02760a Mar 10, 2020
@rayanimesh
travis: Add compile check for all supported platforms
Loading status checks…
972c3aa
@rayanimesh rayanimesh force-pushed the rayanimesh:travis-compile-check branch from c02760a to 972c3aa Mar 10, 2020
@rayanimesh
Copy link
Contributor Author

@rayanimesh rayanimesh commented Mar 10, 2020

I have added set -e and this fails compilation for go 1.11 and linux/riscv64 platform.

go 1.11 release notes mentions that GOARCH values riscv andriscv64 are introduced but not supported by compiler.
Quoting :

The main Go compiler does not yet support the RISC-V architecture but we've reserved the GOARCH values "riscv" and "riscv64", as used by Gccgo, which does support RISC-V. This means that Go files named *_riscv.go will now also be ignored by Go tools except when those GOOS/GOARCH values are being used.

We should ignore GOARCH values in ['ricsv', 'riscv64'] for go 1.11.x in out test case?
@julienschmidt
Copy link
Member

@julienschmidt julienschmidt commented Mar 10, 2020

At least now we know that it works 😄
Yes, I don't think there is any better solution than ignoring those values for Go 1.11
@rayanimesh
Copy link
Contributor Author

@rayanimesh rayanimesh commented Mar 11, 2020

Checking return code of GOOS=${GOOS} GOARCH=${GOARCH} go tool compile -V before running go build is better way of handing above case.

Found this implementation in this comment.

Updated test.
@julienschmidt
julienschmidt reviewed Mar 11, 2020
View changes
.travis/complie_check.sh Outdated
GOOS=$(echo ${dist} | cut -d "/" -f 1)
GOARCH=$(echo ${dist} | cut -d "/" -f 2)
set +e
echo "Checking compile support for ${GOOS}/${GOARCH}"

This comment has been minimized.

Sign in to view
@julienschmidt

julienschmidt Mar 11, 2020
Member

This seems unnecessary. We already get 1 line of output for every GOOS/GOARCH pair. Either that it is skipped or that we're attempting a build.

This comment has been minimized.

Sign in to view
@rayanimesh

rayanimesh Mar 11, 2020
Author Contributor

This has been updated to print one statement per platform.

@julienschmidt julienschmidt added this to the v1.6.0 milestone Mar 11, 2020
@rayanimesh rayanimesh force-pushed the rayanimesh:travis-compile-check branch 2 times, most recently from 256fbff to 96d22ca Mar 11, 2020
@rayanimesh
travis: Check compile support for platform before build
Loading status checks…
e2b4b5b 
`linux/riscv64` platform was added in `go 1.11` but compile was not
supported. Therefore buidling with `GOOS=linux GOARCH=riscv64` for `go
1.11` throws error.

This check handles above case.
@rayanimesh rayanimesh force-pushed the rayanimesh:travis-compile-check branch from 96d22ca to e2b4b5b Mar 11, 2020
@julienschmidt
julienschmidt approved these changes Mar 11, 2020
View changes
Copy link
Member

@julienschmidt julienschmidt left a comment

LGTM. Thanks!
@julienschmidt julienschmidt merged commit 681ffa8 into go-sql-driver:master Mar 11, 2020
3 checks passed
3 checks passed
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 81.634%
Details
sthagen added a commit to sthagen/mysql that referenced this pull request Mar 12, 2020
@sthagen
Merge pull request #12 from go-sql-driver/master
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
77e0aa2 
travis: Add compile check for all supported platforms (go-sql-driver#1070)
@rayanimesh rayanimesh deleted the rayanimesh:travis-compile-check branch Mar 30, 2020
@DouglasPioStudiare DouglasPioStudiare mentioned this pull request May 7, 2020
Closed
tz70s added a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
@rayanimesh @tz70s
travis: Add compile check for all supported platforms (go-sql-driver#…
e580c7f 
…1070)

Implements a Travis CI task that checks if the driver compiles on all platforms supported by Go.

Fixes go-sql-driver#1050
tz70s added a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
@rayanimesh @tz70s
travis: Add compile check for all supported platforms (go-sql-driver#…
99589c5 
…1070)

Implements a Travis CI task that checks if the driver compiles on all platforms supported by Go.

Fixes go-sql-driver#1050
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shogo82148 shogo82148

@julienschmidt julienschmidt

Assignees
No one assigned
Labels
testing
Projects
None yet
Milestone
v1.6.0
Linked issues

Successfully merging this pull request may close these issues.

Travis Compile Check for all supported platforms
3 participants
@rayanimesh @julienschmidt @shogo82148
You can’t perform that action at this time.