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: create /usr/local/bin directory if it does not exist on binary install #3425

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

cdrage
Copy link
Contributor

@cdrage cdrage commented Aug 3, 2023

fix: create /usr/local/bin directory if it does not exist on binary install

What does this PR do?

  • On macOS and Linux we install the binary to /usr/local/bin. However,
    we do not check to see if it exists or create the directory if it it
    does not exist.
  • This PR updates the installBinaryToSystem function to check and create
    the bin directory
  • Added tests
  • No updates to user UI (this all happens behind the scenes)

Screenshot/screencast of this PR

Screen.Recording.2023-08-03.at.1.50.02.PM.mov

What issues does this PR fix or reference?

Closes #3309

How to test this PR?

  1. Modify the localBinDir const to /usr/local/bin/foobar or a random
    test folder that's non-existant
  2. "Install" Compose using the normal functionality
  3. Check the directory that the binary has been installed.

Signed-off-by: Charlie Drage charlie@charliedrage.com

@cdrage cdrage requested review from benoitf and a team as code owners August 3, 2023 17:52
@cdrage cdrage requested review from jeffmaury and lstocchi and removed request for a team August 3, 2023 17:52
Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

LGTM

extensions/compose/src/cli-run.ts Outdated Show resolved Hide resolved
@cdrage
Copy link
Contributor Author

cdrage commented Aug 4, 2023

Thanks @lstocchi I've updated it with the correct if statement as well as added a test for Linux as well. Good catch! I totally missed that.

install

### What does this PR do?

* On macOS and Linux we install the binary to /usr/local/bin. However,
  we do not check to see if it exists or create the directory if it it
  does not exist.
* This PR updates the installBinaryToSystem function to check and create
  the bin directory
* Added tests
* No updates to user UI (this all happens behind the scenes)

### Screenshot/screencast of this PR

<!-- Please include a screenshot or a screencast explaining what is doing this PR -->

### What issues does this PR fix or reference?

<!-- Please include any related issue from Podman Desktop repository (or from another issue tracker).
-->

Closes containers#3309

### How to test this PR?

1. Modify the `localBinDir` const to `/usr/local/bin/foobar` or a random
   test folder that's non-existant
2. "Install" Compose using the normal functionality
3. Check the directory that the binary has been installed.

<!-- Please explain steps to reproduce -->

Signed-off-by: Charlie Drage <charlie@charliedrage.com>
@cdrage
Copy link
Contributor Author

cdrage commented Aug 4, 2023

Rebased + repushed due to #3437 . Now it passes.

@afbjorklund
Copy link
Contributor

afbjorklund commented Aug 5, 2023

There is a "typo" in the summary and the commit message is weird (GitHub template?)

/usr/bin/local -> /usr/local/bin

@benoitf benoitf changed the title feat: create /usr/bin/local directory if it does not exist on binary install feat: create /usr/local/bin directory if it does not exist on binary install Aug 7, 2023
Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM

Seems fix: rather than feat:

Signed-off-by: Charlie Drage <charlie@charliedrage.com>
@cdrage
Copy link
Contributor Author

cdrage commented Aug 8, 2023

Updated based on commands 💯

@cdrage cdrage changed the title feat: create /usr/local/bin directory if it does not exist on binary install fix: create /usr/local/bin directory if it does not exist on binary install Aug 8, 2023
@cdrage cdrage merged commit 3a47ecb into containers:main Aug 10, 2023
8 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.3.0 milestone Aug 10, 2023
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.

Installing docker-compose system wide fails on macOS 13
6 participants