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

Eclipse commandlet doesn't install Java #265

Closed
jdiazgon opened this issue Oct 1, 2019 · 14 comments
Closed

Eclipse commandlet doesn't install Java #265

jdiazgon opened this issue Oct 1, 2019 · 14 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists
Milestone

Comments

@jdiazgon
Copy link
Member

jdiazgon commented Oct 1, 2019

Expected behavior

When installing Eclipse by running command devon eclipse, it should download the needed JDK so that Eclipse can be installed properly out of the box.

Actual behavior

After running devon eclipse, Eclipse gets installed successfully. However, when installing the plug-ins I get the following errors:

image

IMHO, devon eclipse should call devon java so that this error does not occur. Eclipse without Java does not make sense, so anyway we should force the installation of a JDK.

Steps to reproduce (bug)

  1. Don't run devon java
  2. Run devon eclipse

Comments

Removing Eclipse, running devon java and devon eclipse afterward, fixed the issue

Affected version:

  • OS: Windows
  • Browser: Chrome
@jdiazgon jdiazgon added the bug Something isn't working label Oct 1, 2019
@markusschuh
Copy link
Member

Related to #166, where the java dependency of eclipse - besides other dependencies - was respected in such a way, that a missing java will be installed, but only when explicitly devon eclipse setup is called, see here.
Also, in default case, setup (from base directory) should be called, which should trigger the java setup anyway.

So the remaining question is IMHO, why the java setup is not also called, when the users just executes devon eclipse? ould it be a performance problem, if those scripted checks are called on every run of eclipse?

@jdiazgon
Copy link
Member Author

jdiazgon commented Oct 2, 2019

why the java setup is not also called, when the users just executes devon eclipse?

I completely agree with you. From a user point of view, devon eclipse should install correctly Eclipse. This means that it should download all needed dependencies (like Java)

If this logic was implemented because of performance issues, then I propose the following:

> devon eclipse

Eclipse has not yet been installed on your computer.
Please run "devon eclipse setup" to install it.

@hohwille
Copy link
Member

hohwille commented Oct 14, 2019

I do not fully understand what is going on here:

  1. devon-ide has a setup. Initially users will call setup[.bat] at any later time they can also do updates (devon ide update). Also one can install individual tools even if they are not planned to be used by the project according to their settings (e.g. devon eclipse setup).
  2. next there is the usage of the ide after it has been setup (e.g. devon eclipse). This currently does not recursively validate the installation but expects that a setup has been performed and the installation is sane.

Can you

  • confirm that everything works fine if the setup has been perfromed?
  • clarify if your expecation really is that every devon command should recursively verify the installation of all required tools and reinstall as needed? If so this is a general change of the design and should not be addressed by a PR that only considers eclipse. Please note that this currently also conflicts with requirements from @maybeec who wanted to have the most little overhead and logging output so devon commands are fast and do not spam the log output.

Please note that Eclipse commandlet doesn't install Java is not correct. The setup and devon eclipse setup will check for java and install it in case it is missing.

@hohwille
Copy link
Member

https://github.com/devonfw/ide/blob/master/documentation/setup.asciidoc
What is the setup good for if we expect that the users do not follow the installation process and just ignore it?

@jdiazgon
Copy link
Member Author

jdiazgon commented Oct 14, 2019

Hi @hohwille, thanks for your response. Let me explain myself:

next there is the usage of the ide after it has been setup (e.g. devon eclipse). This currently does not recursively validate the installation but expects that a setup has been performed and the installation is sane.

I completely agree that running devon eclipse should expect the installation to be sane. The problem is that if I have a clean ide and I run devon eclipse, then the ide will automatically install Eclipse (and the JDK is not yet installed). Please see the following code from Eclipse script:

function doSetup() {
  if [ -n "${1}" ]
  then
    doDevonCommand java -q setup
  fi
  if [ -n "${1}" ] || [ ! -d "${ECLIPSE_HOME}" ]
  then
  # Now it is going to install Eclipse...

As you can see, if ${ECLIPSE_HOME} is not set, it will download Eclipse. Following the convention that devon eclipse expects a sane installation, to me this seems incorrect. I think we should remove the first if condition:

function doSetup() {
  if [ -n "${1}" ] || [ ! -d "${ECLIPSE_HOME}" ]
  then
  doDevonCommand java -q setup
  # Now it is going to install Eclipse...

What is the setup good for if we expect that the users do not follow the installation process and just ignore it?

Maybe I'm wrong, but I have not found any part in the documentation where it says that you must add setup to install new software properly, like devon eclipse setup. I think it should be documented.

@markusschuh
Copy link
Member

markusschuh commented Oct 14, 2019

The more complex a piece of software - and devon-ide is more than complex enough - the higher the risk, that some unexpected state will exist later.

I see it at least two times inside the documentation, that setup needs to be called during first installation. But I doubt, this is enough. The devon-ide should not blindly rely on that everything has happened during that setup as expected.

It plays no role, if the user has missed this step, or mix-up the setup with another setup or whatever. devon-ide already contains some additional verification tests here and there, which verify some property before another action is taken. So why not just add another check, which verifies that at least any java is in path before external eclipse is called? A statement like e.g, the one, @jdiazgon has proposed above:

Eclipse has not yet been installed on your computer.
Please run "devon eclipse setup" to install it.

seems far more better to me - since it contains an indication for at least a workaround fix - than the more mystic Eclipse pop-up, that was reported with this issue.

@hohwille
Copy link
Member

I am always trying to challenge requirements and feedback to get to the root of the actual problem. So far still I see some missunderstanding here:

I have not found any part in the documentation where it says that you must add setup to install new software properly, like devon eclipse setup. I think it should be documented.

So if @jdiazgon has successfully run setup there error should have never occurred. As I am sure he is not making up the issue something odd happend in his case that we have not yet been able to trace down.
@jdiazgon the setup[.bat] script installs all the software initially. So after that you should have software/java, software/maven, software/eclipse, etc. properly populated. However, as it seems in your case software/java was entirely missing. We would actually need the logs of the setup to find what went wrong. Probably we need to always log this to the logs directory so that we can request that for analysis from users (may be easily doable by adding a tee pipe to doEcho, etc.). So when you are saying that you have run setup but when you run devon eclipse it is failing as java is not installed then something already went wrong before and that will not be fixed by adding the change proposed by your PR. IMHO we still need to figure out why that happened on your machine. Can you reproduce this error?

I am also willing to change devon-ide such that every commandlet will always recursively verify the installation and install whatever is missing. However, this is a decision that needs to be taken with care and with a plan aligned to our roadmap.
Futher, we have a large community and user-base. As already pointed out there are issues from other users with requirements and expectations that conflict with such approach (e.g. #177).
As @maybeec and @jdiazgon come from the same team I therefore propose that they talk to each other to align their requirements. I am also happy to assist or even setup a call on this if desired.
After all I completely agree that reducing problems that come from users that did not read the documentation at all via more automation and self-curing is good and means less support and maintenance effort for us.
So IMHO this issue is nothing to do as a quick shot but needs some communication and alignment. As we are heading towards a final release we need to postpone this.

@jdiazgon
Copy link
Member Author

Can you reproduce this error?

After double-clicking setup.bat. this is the output I get:

Setting up your devon-ide in C:\Users\jdiazgon\Desktop\temp\devon-ide
scripts/environment-project: line 5: /dev/fd/62: No such file or directory
devon-ide has environment variables have been set for /c/Users/jdiazgon/Desktop/temp/devon-ide in workspace main
/c/Users/jdiazgon/Desktop/temp/devon-ide/scripts/environment-project: line 5: /dev/fd/62: No such file or directory
Missing your settings at  and no SETTINGS_URL is defined.
Further details can be found here:
https://github.com/devonfw/devon-ide/wiki/settings
Please contact the technical lead of your project to get the SETTINGS_URL for your project.
In case you just want to test devon-ide you may simply hit return to install default settings.

Settings URL:

******** ATTENTION ********
ERROR: missing arguments for doInstall:  https://github.com/devonfw/devon-ide-settings.git devon-ide-settings
We are sorry for the inconvenience. Please check the above errors, resolve them and try again.
The operation completed successfully.
The operation completed successfully.
The operation completed successfully.
devon-ide has environment variables have been set for /cygdrive/c/Users/jdiazgon/Desktop/temp/devon-ide in workspace main
Setup of devon-ide completed
Press any key to continue . . .

Are the missing default settings the ones that force the creation of software/java, software/maven, software/eclipse...?

I was not aware that setup.bat should create all those software folders. That logic seems completely correct for me, we just need to find out why it didn't work for me.

If you think this is the real problem, I can close this issue and create a new one.

@markusschuh
Copy link
Member

The above line

scripts/environment-project: line 5: /dev/fd/62: No such file or directory`

inside the ouptut is an indication, that the setup / devon scripts, that run here, still contain the #262 issue. This was such severe (no set of customized environment variables loaded at all) that it causes more severe failures later on. So if scripts >=beta23 are used, the issue discussed here, won't longer appear. (As long as the ide components in $HOME are also new enough - which should be fixed in >=beta24 )

The part, that I am concerned of - and which is related to this issue here - is just, why such a severe side effect ( no environment variable set due to some unforeseen incompatibility between one of several supported runtime environments and the used bashisms) isn't checked later in the scripts and has run into a fatal stop. I'd say, a DEVON_IDE_HOME with empty value were a clear enough signal for the later on started "command"-scripts to stop execution with a n error.

@hohwille
Copy link
Member

@jajimene thanks for the feedback as it reveals another error:
https://github.com/devonfw/devon-ide-settings.git should actually be https://github.com/devonfw/ide-settings.git
See #142 where we seemed to have missed out this little but very important detail.
Excellent finding that I will fix asap. Thanks so much for pointing me to this problem.

BTW: The last time I tested it worked as my git client seems to follow redirects and github was still redirecting the renamed URL.

@hohwille
Copy link
Member

hohwille commented Oct 18, 2019

Just checked but it was already fixed:

SETTINGS_URL="https://github.com/devonfw/ide-settings.git"

So all errors derive from an old version that was used. I was predicting that #142 will cause some problems. Fine that we did not have to rename the CLI command, etc. So hopefully all is working from beta24 on.
Anyhow I would suggest to create a new issue for the requirement to always check and install dependent tools transitively to avoid errors and increase UX (e.g. a JS only project could configure tools to only vscode, nodejs, npm and ng - now if someone in that project wants to call devon eclipse it would then install both java and eclipse. What we also have to be aware is that if you accidentally launch devon eclipse you will start to download and install all the tools. We can still consider to do an interactive confirmation before starting the installation if not triggered via devon * setup.)
In case we otherwise confirm that with all the fixes in place everything works as expected, then we should be fine to close this issue.

@hohwille
Copy link
Member

It would be great if you @jdiazgon could retest this with the latest beta24 and give feedback.
I would close this issue if we get positive feedback or if there is no response for a while assuming that all is fixed.

@jdiazgon
Copy link
Member Author

Completely fixed! Works like a charm :)

@hohwille hohwille added the duplicate This issue or pull request already exists label Dec 17, 2019
@hohwille hohwille added this to the rejected milestone Dec 17, 2019
@hohwille
Copy link
Member

See #281 for the implications of the change. It is available from version 3.2.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants