这是indexloc提供的服务，不要输入任何密码
Skip to content

Add git:unlock command #4465

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 21 commits into from
Mar 12, 2021
Merged

Add git:unlock command #4465

merged 21 commits into from
Mar 12, 2021

Conversation

@Akirtovskis
Copy link
Contributor

@Akirtovskis Akirtovskis commented Mar 7, 2021 •
edited
Loading

Related issue : #4458

To solve the cloning issue :

  • added git:unlock command
  • called within git:sync so on every next build clone directory is removed and build continues
  • added error handling if no tty is present (?)
  • removed cloned directory check from fn-git-clone as we now have check for that in git:sync

Questions that I still have :

  • when trying to run git:unlock app separately from vm it is not working and I see git:unlock is not a dokku command. Is
    there something obviuos I missed in command creation flow?
  • is this good error message for no tty cases?
    dokku_log_fail "Missing tty, try running ssh -t which will allocate new tty for you"

@josegonzalez
Copy link
Member

josegonzalez commented Mar 8, 2021

The unlock shell script needs to be executable.

I would just error with a message telling the user to call git:unlock if no tty is present.

@Akirtovskis
Copy link
Contributor Author

Akirtovskis commented Mar 10, 2021 •
edited
Loading

@josegonzalez Thanks for feedbacks. Fixed both issues and successfully tested git:unlock functionality on local.

Edit : only question left is how to fix the failing pipeline?

josegonzalez
josegonzalez reviewed Mar 10, 2021
View reviewed changes
Copy link
Member

@josegonzalez josegonzalez left a comment

Choose a reason for hiding this comment

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

Can you also write a test? Should just mock creating the clone directory.

The test might fail on gitlab because they don't support tty, so no need to check for the inline unlock, just want to know that the regular git:unlock command does what we expect.

@Akirtovskis
Copy link
Contributor Author

Akirtovskis commented Mar 11, 2021 •
edited
Loading

@josegonzalez added the prompt and also --force flag for cases where it would make sense to bypass the prompt.

Added tests also, but when I was running them they are failing with :

(git) git:unlock [succes]
   (from function `global_setup' in file test_helper.bash, line 23,
    from function `setup' in test file git_6.bats, line 6)
     `global_setup' failed with status 127
   /Users/arturs/arturs/dev/dokku/tests/unit/test_helper.bash: line 23: free: command not found
   /Users/arturs/arturs/dev/dokku/tests/unit/test_helper.bash: line 216: dokku: command not found
   /Users/arturs/arturs/dev/dokku/tests/unit/test_helper.bash: line 37: dokku: command not found

Same is happening on other git test files as well and I am not sure how to fix that.

Edit regarding last linting issue, I tried running shfmt -w file-name and it doesn't do anything while pipeline spits out an error.

@josegonzalez
Copy link
Member

josegonzalez commented Mar 11, 2021

Tests cant be run on OSX, they need to run in a Linux VM (the included one is fine).

@Akirtovskis
Copy link
Contributor Author

Akirtovskis commented Mar 12, 2021

@josegonzalez git:unlock command works as expected now and I even managed to make it cleaner by moving out the fn-git-remove-clone-folder to a separate function.

But I would still appreciate some help with tests and fixing the lint issue.

@josegonzalez
Copy link
Member

josegonzalez commented Mar 12, 2021

Gonna take over the PR from here.

@josegonzalez josegonzalez merged commit 3ab23dd into dokku:master Mar 12, 2021
@josegonzalez josegonzalez mentioned this pull request Mar 12, 2021
Closed
github-actions bot pushed a commit that referenced this pull request Mar 12, 2021
Release 0.24.2
4662ac2 
# History

## 0.24.2

Install/update via the bootstrap script:

```shell
wget https://raw.githubusercontent.com/dokku/dokku/v0.24.2/bootstrap.sh
sudo DOKKU_TAG=v0.24.2 bash bootstrap.sh
```

### Bug Fixes

- #4473: @josegonzalez Add the correct log mount for app logs

### Other

- #4465: @Akirtovskis Add git:unlock command
- #4470: @dependabot-preview[bot] chore(deps): bump socket.io from 3.1.2 to 4.0.0 in /tests/apps/.websocket.disabled
- #4462: @dependabot-preview[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 189 to 190 in /tests/apps/php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@josegonzalez josegonzalez josegonzalez left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Akirtovskis @josegonzalez