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: ensure asdf-plugin-manager works in expected worktree #56

Merged

Conversation

airtonix
Copy link
Contributor

@airtonix airtonix commented Jun 18, 2024

fixes #55

--git-dir doesn't work as by itself, so also include --worktree

@airtonix airtonix requested a review from a team as a code owner June 18, 2024 23:13
scripts/lint.bash Outdated Show resolved Hide resolved
.editorconfig Outdated Show resolved Hide resolved
cli/asdf-plugin-manager.sh Outdated Show resolved Hide resolved
@aabouzaid aabouzaid removed the request for review from a team August 25, 2024 13:39
@its-raj21
Copy link

its-raj21 commented Aug 28, 2024

@aabouzaid, Any idea, when this will be merged.? we are also facing this issue if we aren't using the latest commit ref for the plugins.

Also I have tested this fix by downloading the script and it helps to resolve the issue.

@aabouzaid aabouzaid changed the title fix: ensure checkout_plugin_ref only operates in expected worktree fix: ensure asdf-plugin-manager only operates in expected worktree Sep 23, 2024
@aabouzaid aabouzaid changed the title fix: ensure asdf-plugin-manager only operates in expected worktree fix: ensure asdf-plugin-manager works in expected worktree Sep 23, 2024
@aabouzaid aabouzaid added the bug Something isn't working label Sep 23, 2024
@aabouzaid aabouzaid merged commit 309be80 into asdf-community:main Sep 23, 2024
5 checks passed
@aabouzaid
Copy link
Collaborator

I've fixed the issue, which will be included in the next release.

Comment on lines 79 to +81
plugin_name="${1}"
plugin_ref="${2}"
git --git-dir "${PLUGINS_REPOS_DIR}/${plugin_name}/.git" checkout "${plugin_ref}" -q
git_dir="${PLUGINS_REPOS_DIR}/${plugin_name}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw removing local now means you're reassigning a global variable, which these are not. just fyi.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's only a problem if another global var has the same name.
But in general, I will review all functions vars and make them local to avoid any issues in the future.

@airtonix airtonix deleted the fix/55-dont-modify-repo-in-pwd branch September 24, 2024 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

add-all is modifying my git repo
3 participants