-
Notifications
You must be signed in to change notification settings - Fork 88
fix(sharedmemory): Restrict package to supported platforms #299
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #299 +/- ##
==========================================
- Coverage 75.32% 75.28% -0.05%
==========================================
Files 33 33
Lines 6501 6501
==========================================
- Hits 4897 4894 -3
- Misses 1322 1323 +1
- Partials 282 284 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // Update this variable with the release tag before pushing the tag | ||
| // This value is written to the prelogin and login7 packets during a new connection | ||
| const driverVersion = "v1.9.4" | ||
| const driverVersion = "v1.9.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, as you like. It would be perfect for us to get a release before Dec 3rd to include the feature into our release but it's not a must...
|
what is the work required to fix the npipe package to work on arm? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a build issue for windows/arm64 by restricting the sharedmemory package to only supported Windows architectures (386 and amd64), following the same approach as PR #298 for the namedpipe package. The restriction is necessary because the underlying internal/gopkg.in/natefinch/npipe.v2 dependency only supports these architectures.
- Replaced runtime OS checks with compile-time build constraints
- Restricted sharedmemory package to
windows && (amd64 || 386)platforms - Bumped driver version to v1.9.5
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| version.go | Bumped driver version from v1.9.4 to v1.9.5 |
| sharedmemory/stub.go | Removed old stub file with runtime checks |
| sharedmemory/sharedmemory.go | Removed old file that used runtime checks in init() |
| sharedmemory/sharedmemory_windows.go | Added build constraints for windows && (amd64 || 386), moved type definition and init function from deleted files |
| sharedmemory/sharedmemory_others.go | Added empty stub file for unsupported platforms with appropriate build constraints |
shueybubbles
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
@shueybubbles I have no idea to be honest. ;-) We are just using this driver to connect to MS-SQL database instances but I'm not sure on how this works under the hood. Edit: Seems like e.g. this fork uses arch-independent syscalls which might support arm64, but I've not tested this nor do I know how the underlying library works... |
Similar to issue #297 building the driver for
windows/arm64fails when importingsharedmemorydue to usinginternal/gopkg.in/natefinchwhich does not support the arm64 platform on Windows.This PR applies the same measures as in PR #298 to the
sharedmemorypackage restricting it to building onwindowsfor386andamd64only. Furthermore, the PR bumps the version tov1.9.5.