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

completions: zfs completions broken due to read-only variable #9705

Closed
ammgws opened this issue Apr 3, 2023 · 13 comments
Closed

completions: zfs completions broken due to read-only variable #9705

ammgws opened this issue Apr 3, 2023 · 13 comments
Labels
bug Something that's not working as intended completions
Milestone

Comments

@ammgws
Copy link
Contributor

ammgws commented Apr 3, 2023

# fish --version
fish, version 3.6.1
# zfs load-key r[TAB]: _: cannot overwrite read-only variable
/usr/share/fish/functions/__fish_is_zfs_feature_enabled.fish (line 12): 
    echo $feature_name | read -l _ _ state _
    ^
in function '__fish_is_zfs_feature_enabled' with arguments 'feature@bookmarks'
	called on line 344 of file /usr/share/fish/completions/zfs.fish
from sourcing file /usr/share/fish/completions/zfs.fish

(Type 'help read' for related documentation)
read: _: cannot overwrite read-only variable
/usr/share/fish/functions/__fish_is_zfs_feature_enabled.fish (line 12): 
    echo $feature_name | read -l _ _ state _
    ^
in function '__fish_is_zfs_feature_enabled' with arguments 'feature@bookmarks'
	called on line 508 of file /usr/share/fish/completions/zfs.fish
from sourcing file /usr/share/fish/completions/zfs.fish

(Type 'help read' for related documentation)
read: _: cannot overwrite read-only variable
/usr/share/fish/functions/__fish_is_zfs_feature_enabled.fish (line 12): 
    echo $feature_name | read -l _ _ state _
    ^
in function '__fish_is_zfs_feature_enabled' with arguments 'feature@extensible_dataset'
	called on line 548 of file /usr/share/fish/completions/zfs.fish
from sourcing file /usr/share/fish/completions/zfs.fish

(Type 'help read' for related documentation)
@ammgws
Copy link
Contributor Author

ammgws commented Apr 3, 2023

Huh, seems like this was coincidentally fixed in 85504ca but seems like that didn't make the cut for the 3.6.1 release.

@zanchey zanchey changed the title completions: zsh completions broken due to read-only variable completions: zfs completions broken due to read-only variable Apr 3, 2023
@faho faho added this to the fish 3.6.2 milestone Apr 3, 2023
@faho
Copy link
Member

faho commented Apr 3, 2023

Closing this with a 3.6.2 milestone so we'd add it if we make a 3.6.2 release - other than that it's already fixed so the next full release will include it.

I do consider this to be a good reason to cut a 3.6.2 release, fwiw.

@faho faho closed this as completed Apr 3, 2023
@faho faho added bug Something that's not working as intended and removed enhancement labels Apr 3, 2023
@bakhtiyarneyman
Copy link

Will there be a 3.6.2?

@broizter
Copy link

broizter commented Jun 28, 2023

Will there be a 3.6.2?

If you don't wanna wait for a release you can fix the bug by replacing the contents of /usr/share/fish/functions/__fish_is_zfs_feature_enabled.fish with this

function __fish_is_zfs_feature_enabled -a feature target -d "Returns 0 if the given ZFS feature is available or enabled"
    type -q zpool
    or return
    set -l pool (string replace -r '/.*' '' -- $target)
    set -l feature_name ""
    if test -z "$pool"
        set feature_name (zpool get -H all 2>/dev/null | string match -r "\s$feature\s")
    else
        set feature_name (zpool get -H all $pool 2>/dev/null | string match -r "$pool\s$feature\s")
    end
    if test $status -ne 0 # No such feature
        return 1
    end
    set -l state (echo $feature_name | cut -f3)
    string match -qr '(active|enabled)' -- $state
    return $status
end

It's what 85504ca does.

@bentolor
Copy link

bentolor commented Jul 30, 2023

@broizter I've adopted the patch mentioned above, but I still get on history search:

~  zfs create z0tank/games -o mountpoint=/home/ben/G/usr/share/fish/functions/__fish_is_zfs_feature_enabled.fish (line 8): Expected end of the statement, but found an incomplete token                                                                                                 So 30 Jul 2023 11:13:39 CEST
        set feature_name (zpool get -H all $pool 2>/dev/null | string match -r "$pool\s$feature\s")
                                                                                                 ^
from sourcing file /usr/share/fish/functions/__fish_is_zfs_feature_enabled.fish
	called on line 1 of file /usr/share/fish/completions/zfs.fish
from sourcing file /usr/share/fish/completions/zfs.fish
source: Error while reading file '/usr/share/fish/functions/__fish_is_zfs_feature_enabled.fish'
__fish_is_zfs_feature_enabled: Befehl nicht gefunden.
/usr/share/fish/completions/zfs.fish (line 344): 
if __fish_is_zfs_feature_enabled 'feature@bookmarks'
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~^
from sourcing file /usr/share/fish/completions/zfs.fish
__fish_is_zfs_feature_enabled: Befehl nicht gefunden.
/usr/share/fish/completions/zfs.fish (line 508): 
if __fish_is_zfs_feature_enabled 'feature@bookmarks'
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~^
from sourcing file /usr/share/fish/completions/zfs.fish
__fish_is_zfs_feature_enabled: Befehl nicht gefunden.
/usr/share/fish/completions/zfs.fish (line 548): 
if __fish_is_zfs_feature_enabled "feature@extensible_dataset"
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~^
from sourcing file /usr/share/fish/completions/zfs.fish
ame                                                      s                                                                                                                                                                                                                                ```

Any hints?

@TwoD
Copy link

TwoD commented Sep 14, 2023

Note that the comment above has a linewrap issue.
The first line should be exactly this:
function __fish_is_zfs_feature_enabled -a feature target -d "Returns 0 if the given ZFS feature is available or enabled for the given full-path target (zpool or dataset), or any target if none given"
and the second line begins with (indented) set -l pool

billimek added a commit to billimek/dotfiles that referenced this issue Oct 7, 2023
@luispabon
Copy link

luispabon commented Dec 8, 2023

I am getting this problem on 3.6.4 on Ubuntu 23.10 (official ppa):

image

These are the contents of the file in question:

function __fish_is_zfs_feature_enabled -a feature target -d "Returns 0 if the given ZFS feature is available or enabled for the given full-path target (zpool or dataset), or any target if none given"
    set -l pool (string replace -r '/.*' '' -- $target)
    set -l feature_name ""
    if test -z "$pool"
        set feature_name (zpool get -H all 2>/dev/null | string match -r "\s$feature\s")
    else
        set feature_name (zpool get -H all $pool 2>/dev/null | string match -r "$pool\s$feature\s")
    end
    if test $status -ne 0 # No such feature
        return 1
    end
    echo $feature_name | read -l _ _ state _
    set -l state (echo $feature_name | cut -f3)
    string match -qr '(active|enabled)' -- $state
    return $status
end

Doesn't seem like 85504ca is in there

@bentolor
Copy link

bentolor commented Dec 8, 2023

Similar to @luispabon I get the same error after upstream update from PPA to 3.6.4.

Obviously I patched /usr/share/fish/functions/__fish_is_zfs_feature_enabled.fish manually before as described in the comments, but it seems to have been overwritten again with a broken version.

@luispabon
Copy link

luispabon commented Dec 8, 2023

There might be an issue with the ppa's packaging, the file in the repo is completely different to the file installed by the .deb on the ppa:

https://github.com/fish-shell/fish-shell/blob/3.6.4/share/functions/__fish_is_zfs_feature_enabled.fish

The above was wrong, the file in 3.6.4 master is completely different to this here file in master:

https://github.com/fish-shell/fish-shell/blob/master/share/functions/__fish_is_zfs_feature_enabled.fish

Looks like the changes were merged back in august to master, perhaps 3.6.x releases from a branch instead?

@bentolor
Copy link

bentolor commented Dec 8, 2023

@luispabon You're right: 85504ca is only in master while the last commit of 3.6.4 before release b2ef44a is in Integration_3.6.4 and Integration_3.7.0.

It seems 3.6.x and 3.7.x are not including the fix yet. Not sure how the team cherry-picks/selects what goes into which integration branch.

@luispabon
Copy link

I've replaced my local file for the one in master and seems to be working fine.

@ammgws
Copy link
Contributor Author

ammgws commented Dec 9, 2023

@zanchey FYI for next release

@zanchey
Copy link
Member

zanchey commented Dec 23, 2023

2c460cd looks like it's in Integration_3.7.0, am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that's not working as intended completions
Projects
None yet
Development

No branches or pull requests

8 participants