Skip to content

travis: Add compile check for all supported platforms #1070

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.

Sign up for GitHub

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
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
Loading

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 reviewed changes
Copy link
Contributor

@shogo82148 shogo82148 left a comment

Choose a reason for hiding this comment

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

Thanks!

@rayanimesh rayanimesh force-pushed the travis-compile-check branch from ce66897 to c02760a Compare March 10, 2020 13:37
@rayanimesh rayanimesh force-pushed the travis-compile-check branch from c02760a to 972c3aa Compare March 10, 2020 13:43
@rayanimesh
Copy link
Contributor Author

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

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

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 reviewed 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}"
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 travis-compile-check branch 2 times, most recently from 256fbff to 96d22ca Compare March 11, 2020 10:19
@rayanimesh
travis: Check compile support for platform before build
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 travis-compile-check branch from 96d22ca to e2b4b5b Compare March 11, 2020 11:01
julienschmidt
julienschmidt approved these changes Mar 11, 2020
View reviewed changes
Copy link
Member

@julienschmidt julienschmidt left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@julienschmidt julienschmidt merged commit 681ffa8 into go-sql-driver:master Mar 11, 2020
@rayanimesh rayanimesh deleted the travis-compile-check branch March 30, 2020 10:12
@DouglasPioStudiare DouglasPioStudiare mentioned this pull request May 7, 2020
Closed
tz70s pushed 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 pushed 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 shogo82148 left review comments

@julienschmidt julienschmidt julienschmidt approved these changes

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

Successfully merging this pull request may close these issues.

Travis Compile Check for all supported platforms
3 participants
@rayanimesh @julienschmidt @shogo82148