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

Completion script initialisation problems #912

Closed
johann-petrak opened this issue May 15, 2021 · 26 comments · Fixed by #913
Closed

Completion script initialisation problems #912

johann-petrak opened this issue May 15, 2021 · 26 comments · Fixed by #913
Labels

Comments

@johann-petrak
Copy link

Using sdkman SDKMAN 5.11.2+698 on Ubuntu Linux 20.04 and bash 5.0.17(1) with the necessary code in .bashrc to put it in the path.

When I start a new terminal, i.e. invoke a new bash, output like the following sometimes appears on the console:

ommand not found
36m====: command not found

Command '0music' not found, did you mean:

  command 'music' from deb music-bin (1.1.16-1.1build2)

Try: sudo apt install <deb name>


Command '0music' not found, did you mean:

  command 'music' from deb music-bin (1.1.16-1.1build2)

Try: sudo apt install <deb name>


Command '0music' not found, did you mean:

  command 'music' from deb music-bin (1.1.16-1.1build2)

Try: sudo apt install <deb name>

================================================================================: command not found

I did not manage to find out why/when this happens, it appears to be almost random, though it usually happens when opening the first terminal after a restart or after a longer time.

I have observed this behaviour for several weeks now and I am certain this only happens when the SDKMAN-related code is active in .bashrc, never happens when I comment that out.

The "command not found" complaints I get appear to come from strings inside the bash prompt definition and from the names of random directories in my home directory. I have no idea at all what could be going on here, but it became so annoying that I usually deactivate sdkman completely.

So sadly, I cannot provide steps to reproduce because this problem occurs seemingly randomly (but often enough to be very annoying).

@marc0der
Copy link
Member

Hi @johann-petrak,

Looking at this, I think you have something else going on with your system. We don't have any references to 0music in our code, nor do we access anything in your home folder outside of .sdkman. My guess would be that something else is up here.

If the problem persists, please feel free to chat with us on the SDKMAN slack as described in the contributor guidelines.

@johann-petrak
Copy link
Author

johann-petrak commented May 15, 2021

As I said, it provably only ever happens whe sdkman is in the .bashrc - i deliberately observed both settings for several weeks.

It looks as if in the sdkman bash code some variable gets expanded to unexpected results.

@marc0der marc0der reopened this May 15, 2021
@marc0der
Copy link
Member

After chatting on Slack, we discovered an interaction with the update command and the bash completion script. Opening this issue for further investigation.

@johann-petrak
Copy link
Author

johann-petrak commented May 15, 2021

As it turns out, this probably comes from the second line of code that gets included in the .bashrc by the sdkman installation:

source <(sdk completion bash)

This command is supposed to emit bash code that gets sourced on stdout but sometimes, the command sdk completion bash appears to emit other things on stdout.

When enabling debug mode in .sdkman/etc/config the following message is emitted before the actual bash code:

No update at this time. Using existing cache: ant,asciidoctorj,ballerina,bpipe,btrace,ceylon,concurnas,cuba,cxf,doctoolchain,flink,gaiden,gradle,gradleprofiler,grails,groovy,groovyserv,http4k,infrastructor,java,jbake,jbang,jmc,jreleaser,karaf,kotlin,kscript,layrry,leiningen,maven,micronaut,mulefd,mvnd,pomchecker,sbt,scala,spark,springboot,sshoogr,test,tomcat,vertx,visualvm,webtau
Not refreshing version cache now...

This is probably NOT what was causing the original intermittent problem (as the symptoms are different) but it is very likely that the sdk function may occasionally also emit other messages or output other than just the bash code when running sdk completion bash.

For me, as a workaround to test, I have changed the lines in .bashrc to:

[[ -s "/home/johann/.sdkman/bin/sdkman-init.sh" ]] && source "/home/johann/.sdkman/bin/sdkman-init.sh"
source /home/johann/.sdkman/contrib/completion/bash/sdk

I do not understand why the installation procedure does not do it like this in the first place, I do not think there
should ever be the need to execute anything but the code in the completion script to make this work.
So if one could just directly source the completion file, the danger of the sdk completion xxx command emitting something in addition to the code to stdout somewhere immediately would be removed.
It would most likely also increase the speed of starting a new bash/terminal as running the whole sdk command will take longer than directly sourcing that file.

@jochenwierum
Copy link

I have the same problem. Sometimes when I open up the shell, I see "/proc/self/fd/22:1: bad pattern: ^[[1". Since I'm using zsh, the output looks a bit different, but I think the problem is the same.

Here is what i found out:
When opening the shell, zsh does the usual source <(sdk completion zsh).

Normally, the completion snippets will be generated and inserted here. However sometimes sdkman finds updates and prepends a broadcast message to the output. So the output of sdk completion zsh sometimes is:

==== BROADCAST =================================================================
* 2021-05-15: jbake 2.6.7 available on SDKMAN!
* 2021-05-15: webtau 1.41 available on SDKMAN!
* 2021-05-12: micronaut 2.5.3 available on SDKMAN!
================================================================================
#!/usr/bin/zsh

_sdk() {
	case "${CURRENT}" in
	2)
…

This output is then sourced by the shell. While zsh just complains about a "bad pattern", bash replaces the * with the content of your current directory (this is why you see "music" in the error message).

A propper fix would be to disable broadcast messages in the completion subcommand.

Hope I could help :)

@natros
Copy link

natros commented May 16, 2021

I have the same problem with CentOS 6.x

@marc0der
Copy link
Member

We will be releasing a fix for this as soon as possible. In the meanwhile, please reference the file directly from your bash/ZSH profile like @johann-petrak suggested.

@vpavic
Copy link

vpavic commented May 17, 2021

FWIW I'm also affected by this, and the workaround suggested by @johann-petrak appears to be working fine.

One slight tweak that might make it more generally reusable is to leverage the SDKMAN_DIR environment variable which should've been set at that stage, i.e.:

source "$SDKMAN_DIR/contrib/completion/bash/sdk"

Also, in terms of general recommendation for setting up completion, I wonder isn't it more efficient to simply source the completion file directly rather than invoking a command that'll echo the completion?

@johann-petrak
Copy link
Author

Thanks for the fix!

Still curious though: why does the skd installation procedure not just put the code for sourcing the appropriate completion file directly, why does this need to go through the sdk function?

@marc0der
Copy link
Member

Agreed, it would be a better option. We could even source it from within the sdkman-init.sh, with the type of completion based on if it's bash/ZSH. We could also add a feature toggle to switch it on/off.

@marc0der
Copy link
Member

I think we have a tactical fix for now, but I'm going to keep this open so we can do this properly.

@marc0der marc0der reopened this May 17, 2021
@marc0der marc0der changed the title When starting bash: "36m====: command not found" and more error messages Completion script initialisation problems May 18, 2021
@marc0der
Copy link
Member

marc0der commented May 18, 2021

Okay, changes have been made and are on master (and available to try in the beta channel).

All that is required now is to add sdkman_auto_complete=true to the .sdkman/etc/config.

It will automatically sense your shell and source the appropriate completion script.

Please give it a go and let me know if that works.

@johann-petrak
Copy link
Author

Great!
So if I understand correctly this means that by default the only line placed in .bashrc will be the line sourcing sdkman-init.sh because the shell will be auto-detected for completion support?
Will the sdk command "completion" then still be supported/needed and what will it do?

@jochenwierum
Copy link

I can confirm that the current beta version does not produce any warnings when I fire up my shell. Completion works as expected in zsh.

Thank you very much.

@marc0der
Copy link
Member

@johann-petrak yes exactly, but I've done it in such a way that having that snippet in there (for people that have already added it) won't break anything. I've also removed completion as a command in the help output, so it is purely there for compatibility's sake and does nothing.

@jochenwierum excellent stuff, glad to hear that it's working 👍

@vpavic
Copy link

vpavic commented May 19, 2021

After pulling in from the beta channel, all good on my end as well. 👍

BTW @marc0der, now that the completion command is obsoleted, do you think it makes sense to also remove it from the completion file itself? I see it's still present in there, so things are a bit inconsistent with what sdk help offers.

@marc0der
Copy link
Member

Ah, I thought I'd removed it. Yes indeed. I'm also going to remove the sdkman-completion.sh all together.

@christophehenry
Copy link

christophehenry commented May 21, 2021

Hey, I just tried to type sdk completion bash in my terminal right now and, beyond the completion script, sdkman also printed the warning about a new version of sdkman:

ATTENTION: A new version of SDKMAN is available...

The current version is 5.11.4+709, but you have 5.11.0+644.

Would you like to upgrade now? (Y/n): 

I'm not sure if this is related to the current bug but, if that update notice gets sourced, then it would cause a bug resembling to this one.

Edit: ok, I see this was already pointed by this comment. Sorry for the noise.

@marc0der
Copy link
Member

@christophehenry are you seeing this on the beta channel? This is where we've fixed it.

@marc0der
Copy link
Member

I've done some final cleanup on this feature and addressed the completion tweaks as suggested by @vpavic. Please give it a final test before I release this version into the wild.

@vpavic
Copy link

vpavic commented May 22, 2021

I've just pulled the latest from the beta channel (i.e. master+712) and everything looks good to me.

On top of verifying that the completion is set up properly using sdkman_auto_complete=true and without having anything completion specific in my .bashrc, these are the other things that I've tried:

  • completion doesn't offer completion command:
$ sdk 
broadcast   current     env         help        install     offline     uninstall   upgrade     version     
config      default     flush       home        list        selfupdate  update      use         
  • help command doesn't list completion command:
$ sdk help 

Usage: sdk <command> [candidate] [version]
       sdk offline <enable|disable>

   commands:
       install   or i    <candidate> [version] [local-path]
       uninstall or rm   <candidate> <version>
       list      or ls   [candidate]
       use       or u    <candidate> <version>
       config
       default   or d    <candidate> [version]
       home      or h    <candidate> <version>
       env       or e    [init|install|clear]
       current   or c    [candidate]
       upgrade   or ug   [candidate]
       version   or v
       broadcast or b
       help
       offline           [enable|disable]
       selfupdate        [force]
       update
       flush             [archives|tmp|broadcast|version]

   candidate  :  the SDK to install: groovy, scala, grails, gradle, kotlin, etc.
                 use list command for comprehensive list of candidates
                 eg: $ sdk list
   version    :  where optional, defaults to latest stable if not provided
                 eg: $ sdk install groovy
   local-path :  optional path to an existing local installation
                 eg: $ sdk install groovy 2.4.13-local /opt/groovy-2.4.13
  • executing completion command doesn't return error i.e. everything's backwards compatible:
$ sdk completion bash && echo $?
0

@marc0der
Copy link
Member

Great to hear. Also, there will be no unexpected broadcasts or update messages. I'll release it shortly.

@christophehenry
Copy link

@marc0der

are you seeing this on the beta channel?

Well, you also removed the completion command on beta channel so I can't tell but the sdkman_auto_complete option works nicely. Be careful to correctly advertise this when releasing though because this is a breaking change.

Anyway, thank you for your work here. Really helpful! 👍

@marc0der
Copy link
Member

marc0der commented May 24, 2021

@christophehenry The completion command is still there but is now simply noop with 0 return code. This guarantees backward compatibility.

I've simply removed all references to it so it's no longer advertised.

@marc0der
Copy link
Member

This has now been resolved and was released in 5.11.5.

@Abhinav1217
Copy link

@christophehenry The completion command is still there but is now simply noop with 0 return code. This guarantees backward compatibility.

I've simply removed all references to it so it's no longer advertised.

But please advertise that we need to manually modify .sdkman/etc/config . I reinstalled my system and spend entire afternoon trying to figure out why my old workaround sdk completion bash >> sdkman_bash_completion was returning an empty file. I haven't been keeping track of sdkman issue-tracker since my issue #466 was resolved and you were aware of broadcast message issue and working on it.

Maybe in Installation guide, add a section that explains how we can enable completions, or maybe in future release, you can set sdkman_auto_complete=true as default config, so users can get completion out of box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants