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: proto-update-deps target on macOS #1838

Merged

Conversation

nicopernas
Copy link
Contributor

Description

As noted in #1798, the proto-update-deps make target is broken on macOS. The culprit is a sed command that works slightly different in its freebsd implementation.
This PR replaces the sed command in the makefile target with a perl oneliner that is arguably easier to read and it works on both macOS and GNU systems.

This only fixes part of the problem described in #1798 though.


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@colin-axner
Copy link
Contributor

Thanks for fixing this! Testing on a GNU system I get:

 make proto-update-deps
syntax error at -e line 1, near "if  =="
Execution of -e aborted due to compilation errors.
make: *** [Makefile:450: proto-update-deps] Error 255

@nicopernas nicopernas force-pushed the nicopernas/fix-proto-update-deps branch from 3ad31ae to 9bbf5c3 Compare August 1, 2022 11:30
@nicopernas
Copy link
Contributor Author

nicopernas commented Aug 1, 2022

Thanks for fixing this! Testing on a GNU system I get:

 make proto-update-deps
syntax error at -e line 1, near "if  =="
Execution of -e aborted due to compilation errors.
make: *** [Makefile:450: proto-update-deps] Error 255

Yeah, silly me. make was interpolating the $ in the perl code. New revision should do the trick. Sorry for that.

Copy link
Contributor

@seantking seantking left a comment

Choose a reason for hiding this comment

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

Works for me on Ubuntu 👍

@nicopernas nicopernas force-pushed the nicopernas/fix-proto-update-deps branch from 9bbf5c3 to e8a4fff Compare August 1, 2022 11:56
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@faddat
Copy link
Contributor

faddat commented Aug 2, 2022

In a nerdy industry, this is one of the nerdiest PR's I've seen

-- and quite useful, too!

Thanks!

PS: Who wrote the migration docs that included the line "on a gnu/linux machine....."?

epic (at the time, they were the finest/only migration docs. I upgraded several chains following those instructions.)

@colin-axner
Copy link
Contributor

thanks again @nicopernas :)

@colin-axner colin-axner merged commit 5969304 into cosmos:main Aug 2, 2022
@nicopernas nicopernas deleted the nicopernas/fix-proto-update-deps branch August 2, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants