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

feat: Removed coreutils (realpath) from dependencies for MacOS #368

Merged
merged 3 commits into from
Apr 18, 2022

Conversation

MaxymVlasov
Copy link
Collaborator

@MaxymVlasov MaxymVlasov commented Apr 18, 2022

Put an x into the box if that apply:

  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.

Description of your changes

Fixes #336, #349 and #367

How can we test changes

Run any hook on MacOs w/o coreutils

repos:
- repo: https://github.com/antonbabenko/pre-commit-terraform
  rev: 307f48dae422203f31fa888f6cad7d12014ad260
  hooks:

Copy link
Owner

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Why did we have a previous solution in the first place? Why there are so many alternative solutions listed here - https://stackoverflow.com/questions/3572030/bash-script-absolute-path-with-os-x ? :)

@yermulnik
Copy link
Collaborator

Why there are so many alternative solutions listed here

Coz those who use MacOS are sentenced by Apple to use old, outdated, and EOL'ed NIXish software where Apple doesn't want to conform with licensing in favour of their users suffering from incompetence to install additional modern software using simple solutions like Homebrew and use right tools for the right tasks 🤷🏻
So since there are plenty of such people using pre-commit-terraform I feel like Max is right in replacing the "right tool" with a workaround, which looks most reliable among dozens of alternatives.

Copy link
Collaborator

@yermulnik yermulnik left a comment

Choose a reason for hiding this comment

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

Let's make happy those people who miss a lot of useful tools.
Linux:

> dpkg -L coreutils | fgrep /bin/ | awk -F/ '{print $NF}' | sort | uniq | wc -l
104

Homebrew on MacOS:

> brew list --verbose coreutils | fgrep /bin/ | awk -F/ '{print $NF}' | sort | uniq | wc -l
133

@MaxymVlasov
Copy link
Collaborator Author

Why did we have a previous solution in the first place?

Because it run out of the box in Linux and realpath was introduced in terraform_validate by mcdonnnj on 4 Apr 2020 (v1.29.0) in 7694fb9

I recheck repo for realpath - now it not exist

@MaxymVlasov MaxymVlasov marked this pull request as draft April 18, 2022 14:35
@MaxymVlasov MaxymVlasov marked this pull request as ready for review April 18, 2022 14:38
@antonbabenko antonbabenko changed the title fix: No more need coreutils as deps in MacOS feat: Removed coreutils (realpath) from dependencies for MacOS Apr 18, 2022
@antonbabenko antonbabenko merged commit 944a2e5 into master Apr 18, 2022
@antonbabenko antonbabenko deleted the remove_coreutils_as_hooks_dep branch April 18, 2022 16:17
antonbabenko pushed a commit that referenced this pull request Apr 18, 2022
# [1.68.0](v1.67.0...v1.68.0) (2022-04-18)

### Features

* Removed `coreutils` (realpath) from dependencies for MacOS ([#368](#368)) ([944a2e5](944a2e5))
@antonbabenko
Copy link
Owner

This PR is included in version 1.68.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Realpath error with 1.63.0
3 participants