Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Open Che workspace with multiple workspace root #778

Merged
merged 10 commits into from
Feb 25, 2021

Conversation

benoitf
Copy link
Contributor

@benoitf benoitf commented Jun 23, 2020

What does this PR do?

Handle multiple workspace roots
Checks if there is a che.theia-workspace file.
if there is one, returns that file as being a workspace root
if none, generate the file by getting all clonedPath/path of the devfile projects

enhancements:

  • make it optional for now (like if there is a special attribute in devfile, use that mode)
  • track new folders in /projects and add them to the che.theia-workspace file automatically (and if we delete folders)
  • allow to customize/provide different projects to be added in workspace root. Could be useful for a maven project to see only sub-folders/modules

I think we could resolve those items ^^ in a separate issue: by default a workspace starts in OFF multi-root mode - so no breaking changes for the master branch

The current state of the PR - a workspace starts in OFF multi-root mode - so the behavior should be exactly as for master branch.
A user should add the following section to a devfile to turn on multi-root mode:

attributes:
 multiRoot: 'on'

Please let me know if you have a better idea for the attribute and value as marker for multi-root mode - it's easy to change on the current step.

Change-Id: Idfb9370dc503bcfb5504ba37446f7f1ab171b515
Signed-off-by: Florent Benoit fbenoit@redhat.com

What issues does this PR fix or reference?

eclipse-che/che#17212

TODO

  • Adding a workspace folder directly after cloning a project (not before, not during)
  • Align the corresponding plugin API of Theia with VS Code to provide the ability to display progress for views from plugin side (the PR on Theia side)
  • Minor: Replace CheApiService by WorkspaceService
  • Import a debug configuration from the devfile to a multi-root workspace Open Che workspace with multiple workspace root #778 (comment)
  • Fix config storage paths Change config storage paths from /home/theia/home/theia to the corresponding paths che#18548
  • Adapt handling of Readme files to multi-root workspace changes
  • A debug configuration is not available for running
  • Add workspace folders in turn (there is a problem related to plugin API at adding project as workspace folder - a project is not displayed in Projects view when there are few projects are adding as workspace folders at the same time)
  • Investigate if it's possible to export configurations on Workspace level Open Che workspace with multiple workspace root #778 (comment)
  • Open Configurations and Add configuration actions are not working for a multi-root workspace
  • Adapt Happy Path Tests to multi-root workspace changes
  • An ability to switch (turn on and turn off ) multi-root mode on a devfile level

Try it with two go projects, one with go modules, another without:
http://che.openshift.io/f/?url=https://gist.githubusercontent.com/benoitf/57ffa7e39e2e16a6a2f6756a44693e43/raw/5afea100c012fe05e325358f0a58577821a3cb99/golang-example.devfile.yaml

Happy Path Channel

HAPPY_PATH_CHANNEL=next

@benoitf benoitf changed the title Handle multiple workspace root Open Che workspace with multiple workspace root Jun 23, 2020
@che-bot

This comment has been minimized.

@tsmaeder
Copy link
Contributor

I think an approach that computes the workspace roots at the same time as doing the cloning would make it easier keeping projects and workspace roots in sync as we evolve the code. We can also make sure language servers are not seeing half-cloned projects that way. #408 implements that approach and also cleans up the UI a bit. I would recommend we base our work on that PR.

@sunix
Copy link
Contributor

sunix commented Aug 10, 2020

From what i see in the exemple, it's only possible to define one workspace root per project.
But in some usecase, we would like to be able to use 1+ roots in subfolders (a maven project + nodejs) of the same git project)
What about that proposal:
eclipse-che/che#15347 (comment)

Also why adding a intermediate file? che.theia-workspace file

@benoitf
Copy link
Contributor Author

benoitf commented Aug 10, 2020

@sunix: not yet as it requires a new devfile format

Also why adding a intermediate file? che.theia-workspace file

to make it persistent across workspace restart.

@sunix
Copy link
Contributor

sunix commented Aug 10, 2020

to make it persistent across workspace restart.

Ok, I feel like this is all temporary ... waiting for a new devfile format. What are the next steps?

@RomanNikitenko RomanNikitenko force-pushed the devfile-workspace-root branch from de2de5a to 0e2b132 Compare October 7, 2020 11:46
@che-bot

This comment has been minimized.

@RomanNikitenko RomanNikitenko force-pushed the devfile-workspace-root branch from 0e2b132 to 23712a2 Compare October 8, 2020 13:39
@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@RomanNikitenko
Copy link
Member

Faced with the issue related to cloning a project, investigating...

cloning_error

@benoitf
Copy link
Contributor Author

benoitf commented Oct 9, 2020

@RomanNikitenko looks like it was related to my hack "commit"

@RomanNikitenko
Copy link
Member

@benoitf
hack "commit" was on task plugin side (if i'm not mistaken)
current problem is related to cloning
but probably removing the commit somehow broke the behavior...

it's also strange for me because yesterday I tested after removing the commit using my another devfile with theia and che-theia projects and it worked well
today I took a devfile from happy path tests and faced with the issue...

@benoitf
Copy link
Contributor Author

benoitf commented Oct 9, 2020

@RomanNikitenko as you've a launch.json in the workspace root folder, theia try to clone project but it can't as there is already some files there

@RomanNikitenko
Copy link
Member

I think an approach that computes the workspace roots at the same time as doing the cloning would make it easier keeping projects and workspace roots in sync as we evolve the code. We can also make sure language servers are not seeing half-cloned projects that way. #408 implements that approach and also cleans up the UI a bit. I would recommend we base our work on that PR.

@tsmaeder
I tried to apply your idea, so adding a workspace folder directly after cloning a project.
I'm worried about the behavior related to project tree with this approach.
You have not yet added a folder to the workspace message and Add Folder button is displayed while a project is cloning.
Sure in the right bottom area the notification about cloning project process is displayed at the same time.
But I guess the behavior can confuse an user, especially for case of slow connection or a big project.

importing

@tsmaeder
Copy link
Contributor

@RomanNikitenko I agree this can be confusing when there is a long wait before cloning starts. A couple of ideas:

  1. Unbind the UI that shows the "Open Workspace" button.
  2. Maybe open some (closable) modal UI at startup that tells the user cloning will start shortly?

@RomanNikitenko
Copy link
Member

@tsmaeder

I agree this can be confusing when there is a long wait before cloning starts.

To clarify: in my implementation a workspace folder is added when cloning is completed, because you mentioned that:

We can also make sure language servers are not seeing half-cloned projects that way.

@tsmaeder
Copy link
Contributor

To clarify: in my implementation a workspace folder is added when cloning is completed, because you mentioned that:

I understand that...but when the user sees that projects are being added, she knows something is going on; when the IDE just sits there, it's much worse.

@RomanNikitenko
Copy link
Member

I understand that...but when the user sees that projects are being added, she knows something is going on; when the IDE just sits there, it's much worse.

I agree. I just highlighted some details in my previous comment to be sure that we speak about the same things.

@azatsarynnyy
Copy link
Member

Let's try the solution we've agreed on during our standup call, yesterday.
Customize the Explorer in Che Theia to get rid of the existing text + button and display some progress instead, while cloning.

To not spend much time creating something new, we can try to reuse the existing progress indicator - this fast-moving line for example:
Peek 2020-10-28 13-08

... and some text, e.g. Cloning the sources...

@RomanNikitenko
Copy link
Member

I aligned the corresponding plugin API of Theia with VS Code and provided the ability to display progress for views from plugin side, so now we can improve the behavior described here and apply this approach

@che-bot
Copy link
Contributor

che-bot commented Nov 19, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:778
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:778

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@RomanNikitenko
Copy link
Member

[crw-ci-test]

@che-bot
Copy link
Contributor

che-bot commented Nov 19, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:778
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:778

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@benoitf
Copy link
Contributor Author

benoitf commented Nov 19, 2020

[crw-ci-test]

@che-bot

This comment has been minimized.

@che-bot

This comment has been minimized.

@che-bot
Copy link
Contributor

che-bot commented Feb 18, 2021

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:778
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:778

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Feb 18, 2021

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:778
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:778

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Copy link
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

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

After several rounds of testing, I don't see any issues anymore.
I think it's ready now and all other enhancements we could make in further iterations.

@RomanNikitenko
Copy link
Member

I have rebased the branch and added the handling of /home/theia-dev case according to #778 (comment)

@tsmaeder
please take a look at the changes ecbb10b

thanks in advance!

@che-bot
Copy link
Contributor

che-bot commented Feb 19, 2021

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:778
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:778

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@benoitf
Copy link
Contributor Author

benoitf commented Feb 19, 2021

I can't approve the PR as I created it :-)

@azatsarynnyy
Copy link
Member

I can't approve the PR as I created it :-)

@benoitf We'll take this ^ as your approvement 😄

benoitf and others added 10 commits February 24, 2021 17:07
…ts (or what has been configured as ROOT folder)

Checks if there is a che.theia-workspace file.
  if there is one, returns that file as being a workspace root
  if none, generate the file by getting all clonedPath/path of the devfile projects

enhancements:
 1. make it optional for now (like if there is a special attribute in devfile, use that mode)
 2. track new folders in /projects and add them to the che.theia-workspace file automatically (and if we delete folders)
 3. allow to customize/provide different projects to be added in workspace root. Could be useful for a maven project to see only sub-folders/modules

Change-Id: Idfb9370dc503bcfb5504ba37446f7f1ab171b515
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
…file level

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
@RomanNikitenko
Copy link
Member

RomanNikitenko commented Feb 24, 2021

I have resolved the conflicts (the changes for some files were too close to the merged recently PR) and rebased my branch.
I'm going to merge the changes tomorrow morning if there are no objections and the tests are happy.

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #778 (dee2d73) into master (4e9f582) will decrease coverage by 1.63%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #778      +/-   ##
==========================================
- Coverage   58.02%   56.38%   -1.64%     
==========================================
  Files          46       47       +1     
  Lines        2032     2091      +59     
  Branches      333      345      +12     
==========================================
  Hits         1179     1179              
- Misses        843      900      +57     
- Partials       10       12       +2     
Impacted Files Coverage Δ
plugins/workspace-plugin/src/theia-commands.ts 0.00% <0.00%> (ø)
...s/workspace-plugin/src/workspace-folder-updater.ts 0.00% <0.00%> (ø)
...workspace-plugin/src/workspace-projects-manager.ts 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e9f582...dee2d73. Read the comment docs.

@che-bot
Copy link
Contributor

che-bot commented Feb 24, 2021

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:778
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:778

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@RomanNikitenko RomanNikitenko merged commit 0fc5809 into eclipse-che:master Feb 25, 2021
@che-bot che-bot added this to the 7.27 milestone Feb 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants