fix: <plugin update all> to latest commit on default branch for each plugin by jthegedus · Pull Request #800 · asdf-vm/asdf

jthegedus · 2020-09-09T08:34:12Z

Summary

As @joschi pointed out in #799 asdf plugin update --all does not respect the set gitref with respect to branch names. Since we cannot provide a gitref to each plugin during update --all we should update to the latest commit on the ~~branch set by the user on plugin add~~ default branch of the plugin.

Fixes: #799
Fixes: #813

Solution [WIP]

asdf plugin update <name>:

update to latest commit on default branch of plugin repo

asdf plugin update <name> <gitref>:

fetch provided gitref
checkout that gitref

asdf plugin update --all:

for each plugin
update to latest commit on default branch of plugin repo

todo

~~UNTESTED at this time. --git-dir usage with fetch and checkout was not tested by myself~~ tested by @joschi
create test cases for asdf plugin update
- ensure to capture scenarios where gitref was supplied and then subsequently is not on further updates.
create test case which covers the initial bug so we get warned on regressions
update docs and asdf help to better describe asdf plugin update --all behaviour

Other Information

use --git-dir and --work-tree instead of subshell + cd.
- --work-tree has a min git version of 1.7.7.2 according to https://stackoverflow.com/a/9747584/7911479 This shouldn't increase the required git version for asdf usage AFAICT. According to wikipedia the latest patch for that minor was 1.7.12.4 released on 2012-10-17
use full flag names for git --prune and git --update-head-ok
--git-dir --work-tree is prefered over -C due to the lower supported git version

Caveats

It is worth pointint out, that this code no longer assumes master as the default branch, but does assume origin as the default name of the remote.

lib/commands/command-plugin-update.bash

joschi · 2020-09-09T15:16:16Z

It seems to work (tested in a Docker container with ubuntu:20.04):

root@b3fd500f74b7:~# git clone https://github.com/jthegedus/asdf.git --branch fix/update-all-plugins --depth 1
Cloning into 'asdf'...
remote: Enumerating objects: 139, done.
remote: Counting objects: 100% (139/139), done.
remote: Compressing objects: 100% (124/124), done.
remote: Total 139 (delta 8), reused 55 (delta 1), pack-reused 0
Receiving objects: 100% (139/139), 79.95 KiB | 889.00 KiB/s, done.
Resolving deltas: 100% (8/8), done.
root@b3fd500f74b7:~# source $HOME/asdf/asdf.sh
root@b3fd500f74b7:~# asdf info
OS:
Linux b3fd500f74b7 4.19.76-linuxkit #1 SMP Tue May 26 11:42:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

SHELL:
GNU bash, version 5.0.17(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

ASDF VERSION:
v0.8.0-9cafd45

ASDF ENVIRONMENT VARIABLES:
ASDF_DIR=/root/asdf

Oohes nooes ~! No plugins installed
ASDF INSTALLED PLUGINS:

root@b3fd500f74b7:~# asdf plugin add jbang
initializing plugin repository...
Cloning into '/root/.asdf/repository'...
remote: Enumerating objects: 102, done.
remote: Counting objects: 100% (102/102), done.
remote: Compressing objects: 100% (52/52), done.
remote: Total 2326 (delta 59), reused 90 (delta 50), pack-reused 2224
Receiving objects: 100% (2326/2326), 544.52 KiB | 1.65 MiB/s, done.
Resolving deltas: 100% (1059/1059), done.
root@b3fd500f74b7:~# asdf plugin update --all
Updating jbang...
Already on 'main'
Your branch is up to date with 'origin/main'.
root@b3fd500f74b7:~# git --git-dir $HOME/.asdf/plugins/jbang/.git checkout -b new-branch
Switched to a new branch 'new-branch'
root@b3fd500f74b7:~# asdf plugin update --all
Updating jbang...
fatal: couldn't find remote ref new-branch
Already on 'new-branch'
root@b3fd500f74b7:~# asdf plugin update jbang main
Updating jbang...
Switched to branch 'main'
Your branch is up to date with 'origin/main'.
root@b3fd500f74b7:~# asdf plugin update jbang
Updating jbang...
Already on 'main'
Your branch is up to date with 'origin/main'.

jthegedus · 2020-09-18T01:11:05Z

I should have time to finish this off come this weekend, sorry for the delay.

joschi · 2020-10-26T12:44:56Z

@jthegedus Can I somehow help you to finish and merge this PR?

jthegedus · 2020-10-26T21:53:42Z

@joschi yes please, I think the todo list in this PR is a good starting place

jthegedus · 2020-12-18T22:42:54Z

Can probably sneak the fix for #813 in here

jthegedus · 2020-12-19T06:39:15Z

Upon testing the current impl it works for cases where the a specified gitref is a branch, but when it is a commit and HEAD goes into a detached state, ~~it causes a lot of issues~~ subsequent calls result in the creation of a branch called HEAD, which is a bad state. EG, this doesn't work well:

asdf plugin add <name>
asdf plugin update <name> <commit shaX>
asdf plugin update <name>
asdf plugin update <name> <commit shaY> -> HEAD branch

I am working towards a solution that will support both commit and brach refs (tags will be dealth with later).

MetricMike · 2021-01-29T01:15:15Z

Very sorry to @/email here, but this PR has been lingering for over a month and seems to almost address the issue. @jthegedus were you able to make any progress on this and/or still open for help with the commit/HEAD bug?

Github repos are increasingly likely to switch or start with "main" more frequently as time goes on and this PR is going to be especially useful for new plugins. I'm trying to implement asdf-awscli and running into this problem myself :)

jthegedus · 2021-01-29T01:25:53Z

Newer changes to the code this touches makes this harder to manage. Previously updates were via a full git pull of the plugin repo, but some optimisations were made to only pull particular shas which make finding the latest commit on the current branch more difficult. I also want to improve the testing around this as some cases should be caught for regressions.

I will spend some time this weekend to finally push this out. That may just be a revert to the older git pull which we can re-implement the optimisation of in future.

MetricMike · 2021-01-29T01:30:49Z

Thanks so much! It's not a huge blocker, as I can just mirror my changes from main to master for now, but this'll be a nice quality-of-life improvement for future contributions.

…/asdf into fix/update-all-plugins

jthegedus · 2021-02-02T02:53:43Z

So for simplicity sake and to get this over the line I decided to go with the @joschi suggestion of

asdf should use the default branch of the plugin repository for updates

as this is much easier to get working consistently than the other solution I originally committed to

only pull the latest changes of the checked out branch even if it's not master

I was unable to get a solution using the "existing branch of sha" soln that also covers the case I outline in this comment #800 (comment) , that's not to say it's not possible, but I could not.

As non-master repos are becoming common I feel we can get the soln using the remote default branch to users sooner. Should we desire to come back to this we can.

The code:

git --git-dir "$plugin_path/.git" --work-tree "$plugin_path" fetch --prune --update-head-ok origin "$gitref:$gitref"

specifically the usage of --update-head-ok is fairly non-standard usage of git. I am unsure if it is the cause of the following loud outputs from git:

.asdf on  master [⇣!] took 1s 
➜ asdf plugin update firebase 3a97373
Updating firebase...
fatal: couldn't find remote ref 3a97373
Note: switching to '3a97373'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 3a97373 fix: shellcheck download URL and shfmt flags

.asdf on  master [⇣!] took 1s 
➜ asdf plugin update firebase 0496904
Updating firebase...
fatal: couldn't find remote ref 0496904
Previous HEAD position was 3a97373 fix: shellcheck download URL and shfmt flags
HEAD is now at 0496904 feat: firebase tools asdf plugin (#1)

.asdf on  master [⇣!] 
➜ asdf plugin update firebase
Updating firebase...
Previous HEAD position was 0496904 feat: firebase tools asdf plugin (#1)
Switched to branch 'main'
Your branch is up to date with 'origin/main'.

The output of fatal: couldn't find remote ref <ref> is curious given it does then update to that ref 🤷‍♂️

Nonetheless, the solution of checking out the latest sha from the remote's default branch does not result in the issues I outlined in comment #800 (comment)

I will ping again for final reviews when ready.

…or non-branch/tag ref checkouts

jthegedus · 2021-02-02T03:51:46Z

I discovered you can toggle off specific "git advice" logging per command, so have done so with the git checkout which was producing the aforementioned noise and poor logging. The result is now this:

.asdf on  master [⇣!] took 6s 
➜ asdf plugin update firebase 3a97373
Updating firebase...
fatal: couldn't find remote ref 3a97373
HEAD is now at 3a97373 fix: shellcheck download URL and shfmt flags

.asdf on  master [⇣!] took 1s 
➜ asdf plugin update firebase 0496904
Updating firebase...
fatal: couldn't find remote ref 0496904
Previous HEAD position was 3a97373 fix: shellcheck download URL and shfmt flags
HEAD is now at 0496904 feat: firebase tools asdf plugin (#1)

.asdf on  master [⇣!] took 1s 
➜ asdf plugin update firebase        
Updating firebase...
Previous HEAD position was 0496904 feat: firebase tools asdf plugin (#1)
Switched to branch 'main'
Your branch is up to date with 'origin/main'.

It still contains fatal, but correctly tells the user where HEAD previously and now points as it does for branches 💯

… default branch

lib/commands/command-plugin-update.bash

jthegedus · 2021-02-02T22:32:51Z

test/plugin_update_command.bats

-  BASE_DIR=$(mktemp -dt asdf.XXXX)
-  HOME=$BASE_DIR/home
-  ASDF_DIR=$HOME/.asdf
-  git clone -o local "$(dirname "$BATS_TEST_DIRNAME")" "$ASDF_DIR"
-  git --git-dir "$ASDF_DIR/.git" remote add origin https://github.com/asdf-vm/asdf.git
-  mkdir -p "$ASDF_DIR/plugins"
-  ASDF_BIN="$ASDF_DIR/bin"
-
-  # shellcheck disable=SC2031
-  PATH=$ASDF_BIN:$ASDF_DIR/shims:$PATH
-  install_dummy_plugin
-
-  PROJECT_DIR=$HOME/project
-  mkdir $PROJECT_DIR


I believe this code was to set the test env asdf root to have a .git dir with the remote set to https://github.com/asdf-vm/asdf.git to enable testing of plugin repo fetching.

I need to put this functionality back in, but think we can put it in the initial setup_asdf_dir function.

jthegedus · 2021-02-02T22:35:44Z

Though a successful test has run for the core functionality this update adds, I would like to add additional tests before merging for the scenarios I outlined earlier where individual shas are used etc.

Getting close now.

Stratus3D · 2021-02-05T14:02:35Z

Thanks for all your work on this @jthegedus, I'll take a look again as soon as I see new changes!

jthegedus · 2021-02-07T04:51:33Z

Ready for review @Stratus3D @joschi

jthegedus · 2021-02-28T22:05:21Z

I'm fairly confident with this change and will merge in the next day or two. Further testing can be carried out by the people (like myself) who use asdf update --head

Stratus3D · 2021-03-01T19:44:52Z

Tests pass for me locally on OSX 10.15.7.

joschi · 2021-03-02T07:12:26Z

@jthegedus @Stratus3D Thanks for taking care of this and testing it! ❤️

jthegedus added 2 commits September 9, 2020 18:19

fix: update all plugins using branch of head

ede3658

fix: suggested usage of plugin update

9cafd45

jthegedus added the WIP label Sep 9, 2020

jthegedus requested a review from a team as a code owner September 9, 2020 08:34

jthegedus self-assigned this Sep 9, 2020

jthegedus commented Sep 9, 2020

View reviewed changes

lib/commands/command-plugin-update.bash Outdated Show resolved Hide resolved

jthegedus changed the title ~~fix: plugin update all using branch of current HEAD~~ fix: plugin update all using branch of plugins current HEAD Sep 9, 2020

jthegedus added the bug label Sep 9, 2020

chore: cleanup and use git --work-tree for checkout and fetch

04c9c62

jthegedus mentioned this pull request Sep 9, 2020

Clean up dependencies in documentation #511

Closed

jthegedus mentioned this pull request Sep 18, 2020

Versioning of plugins #166

Open

jthegedus mentioned this pull request Dec 17, 2020

add starter plugin version feature (Addresses Issue #166) #234

Closed

jthegedus mentioned this pull request Dec 19, 2020

Notify users of core or plugin updates #725

Open

jthegedus changed the title ~~fix: plugin update all using branch of plugins current HEAD~~ fix: <plugin update all> to latest commit on current branch for each plugin Dec 21, 2020

jthegedus added 6 commits February 1, 2021 18:19

Merge branch 'master' into fix/update-all-plugins

8b62620

test: list test cases for plugin-update

a4e010c

Merge branch 'fix/update-all-plugins' of https://github.com/jthegedus…

d895bb6

…/asdf into fix/update-all-plugins

fix: detect default branch instead of branch of current sha

2baea65

test: remove test file as cmd tests exist elsewhere

67a95b1

refactor: revert to loop as per #803 and #805

15f5659

fix: explicitly toggle off git suggestion about detached head state f…

0d9ab97

…or non-branch/tag ref checkouts

jthegedus added 6 commits February 2, 2021 14:53

revert: use long flag names, remove unused --quiet

410d3f9

fix: #813 fix for while loop

7e95103

test: ensure git executes against plugin .git dir

68e5f61

docs: describe plugin update --all new functionality

4910923

docs: describe plugin update --all new functionality

a106e4a

fix: use git ls-remote instead of relying on local refs to determined…

ab6515c

… default branch

jthegedus commented Feb 2, 2021

View reviewed changes

lib/commands/command-plugin-update.bash Outdated Show resolved Hide resolved

jthegedus added 2 commits February 3, 2021 09:26

fix: improve var name

37af100

test: initial test for plugin update remote branch changes

e2481ec

jthegedus commented Feb 2, 2021

View reviewed changes

feat: handle branches with forward slashes + tests

e07f777

jthegedus removed the WIP label Feb 11, 2021

jthegedus mentioned this pull request Feb 28, 2021

chore: sync main to master jthegedus/asdf-gcloud#38

Merged

Stratus3D approved these changes Mar 1, 2021

View reviewed changes

jthegedus merged commit 456d8e3 into asdf-vm:master Mar 2, 2021

jthegedus deleted the fix/update-all-plugins branch March 2, 2021 06:37

MetricMike mentioned this pull request Mar 5, 2021

Use bundled releases for v2 MetricMike/asdf-awscli#2

Closed

3 tasks

peaceful-james mentioned this pull request Mar 19, 2021

"plugin update" broken by recent PR #899

Closed

jthegedus mentioned this pull request Apr 18, 2021

fix: plugin update portability #924

Merged

sergei-ivanov mentioned this pull request May 17, 2021

Question: When will a new tagged version be published? #921

Closed

theoretick mentioned this pull request Aug 21, 2021

feat: Add versioning to plugin add command #916

Closed

javiergarea mentioned this pull request Dec 11, 2023

fix: plugin test git-ref default to the default branch of the plugin's repository #1694

Merged

Conversation

jthegedus commented Sep 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Solution [WIP]

todo

Other Information

Caveats

Uh oh!

Uh oh!

joschi commented Sep 9, 2020

Uh oh!

jthegedus commented Sep 18, 2020

Uh oh!

joschi commented Oct 26, 2020

Uh oh!

jthegedus commented Oct 26, 2020

Uh oh!

jthegedus commented Dec 18, 2020

Uh oh!

jthegedus commented Dec 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MetricMike commented Jan 29, 2021

Uh oh!

jthegedus commented Jan 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MetricMike commented Jan 29, 2021

Uh oh!

jthegedus commented Feb 2, 2021

Uh oh!

jthegedus commented Feb 2, 2021

Uh oh!

Uh oh!

jthegedus Feb 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jthegedus commented Feb 2, 2021

Uh oh!

Stratus3D commented Feb 5, 2021

Uh oh!

jthegedus commented Feb 7, 2021

Uh oh!

jthegedus commented Feb 28, 2021

Uh oh!

Stratus3D commented Mar 1, 2021

Uh oh!

joschi commented Mar 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jthegedus commented Sep 9, 2020 •

edited

Loading

jthegedus commented Dec 19, 2020 •

edited

Loading

jthegedus commented Jan 29, 2021 •

edited

Loading

jthegedus Feb 2, 2021 •

edited

Loading