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

fix: Enable build in pre-montery machines #1351

Merged
merged 3 commits into from
Apr 27, 2022
Merged

Conversation

mattverse
Copy link
Member

@mattverse mattverse commented Apr 26, 2022

What is the purpose of the change

PR #1268 introduces a build failure bug on Mac OS <= 11 by the package zondax/hid.

$ make build
Makefile:75: RocksDB support is disabled; to build and test with RocksDB support, set ENABLE_ROCKSDB=true
go build -mod=readonly -tags "netgo ledger" -ldflags '-X github.com/cosmos/cosmos-sdk/version.Name=sim -X github.com/cosmos/cosmos-sdk/version.AppName=simd -X github.com/cosmos/cosmos-sdk/version.Version=0.46.0-alpha2-160-gf4d50a989 -X github.com/cosmos/cosmos-sdk/version.Commit=f4d50a989fec8ffbc2e640ad492d40f72580064c -X "github.com/cosmos/cosmos-sdk/version.BuildTags=netgo,ledger" -X github.com/tendermint/tendermint/version.TMCoreSemVer=v0.35.2 -w -s' -trimpath -o /Users/danielwedul/git/cosmos-sdk/build/ ./...
# github.com/keybase/go-keychain
cgo-gcc-prolog:203:11: warning: 'SecTrustedApplicationCreateFromPath' is deprecated: first deprecated in macOS 10.15 - No longer supported [-Wdeprecated-declarations]
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/Security.framework/Headers/SecTrustedApplication.h:59:10: note: 'SecTrustedApplicationCreateFromPath' has been explicitly marked deprecated here
# github.com/zondax/hid
In file included from ../../go/pkg/mod/github.com/zondax/hid@v0.9.1-0.20220302062450-5552068d2266/hid_enabled.go:38:
./hidapi/mac/hid.c:693:34: error: use of undeclared identifier 'kIOMainPortDefault'
        entry = IORegistryEntryFromPath(kIOMainPortDefault, path);
                                        ^
1 error generated.
make: *** [build] Error 2

Although #1268 does remove unnecessary warning logs on Mac OS >12, I think ensuring successful build upon different machines is way more important, which could be done by reverting 1268(which upgrades the zondax/hid to 0.9.1) and use it as it was 0.9.0

cref: cosmos/cosmos-sdk#11364 (comment), cosmos/cosmos-sdk#11365

Brief change log

Testing and Verifying

Locally tested in both mac os >12, mac os <= 11 that #1268 fails on mac os <= 11, while it succeeds on mac OS > 12.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
  • How is the feature or change documented? not applicable

@ValarDragon ValarDragon added the A:backport/v7.x Do not use. backport patches to v7.x branch label Apr 26, 2022
github.com/zondax/hid v0.9.1-0.20220302062450-5552068d2266 // indirect
github.com/zondax/hid v0.9.0 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

Why does changing an indirect dependency not get overriden on go mod updates?

Copy link
Member Author

Choose a reason for hiding this comment

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

wdym?

@ValarDragon
Copy link
Member

Agreed with conclusion of lowering library version!

@codecov-commenter
Copy link

Codecov Report

Merging #1351 (3d8f047) into main (f56fbe5) will decrease coverage by 0.04%.
The diff coverage is 4.00%.

@@            Coverage Diff             @@
##             main    #1351      +/-   ##
==========================================
- Coverage   19.82%   19.77%   -0.05%     
==========================================
  Files         202      202              
  Lines       27685    27718      +33     
==========================================
- Hits         5489     5482       -7     
- Misses      21175    21215      +40     
  Partials     1021     1021              
Impacted Files Coverage Δ
x/gamm/pool-models/balancer/amm.go 17.58% <ø> (ø)
x/gamm/pool-models/stableswap/amm.go 43.28% <0.00%> (-12.49%) ⬇️
x/gamm/pool-models/stableswap/pool.go 0.00% <0.00%> (ø)
x/gamm/client/cli/tx.go 59.50% <100.00%> (-0.69%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07299ea...3d8f047. Read the comment docs.

@ValarDragon ValarDragon merged commit c6ef107 into main Apr 27, 2022
@ValarDragon ValarDragon deleted the mattverse/revert-1268 branch April 27, 2022 15:06
mergify bot pushed a commit that referenced this pull request Apr 27, 2022
(cherry picked from commit c6ef107)

# Conflicts:
#	go.sum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v7.x Do not use. backport patches to v7.x branch
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants