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

chore: update local git config when user changes Che Theia git settings #1319

Merged
merged 11 commits into from
May 25, 2022

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Feb 17, 2022

What does this PR do?

  • When user gitconfig home/theia/.gitconfig is updated with a new value, local gitconfig <project directory>/.git/config is updated with the value automatically.
  • When an existing value is updated in user gitconfig:
    • If the old value is equal with one in local gitconfig, it is updated in the local gitconfig.
    • If the old value is NOT equal with one in local gitconfig, it is NOT updated in the local gitconfig.
  • When a value is deleted in user gitconfig:
    • If the value is equal with one in local gitconfig, it is deleted in the local gitconfig.
    • If the value is NOT equal with one in local gitconfig, it is NOT deleted in the local gitconfig.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#21115

How to test this PR?

run factory:

https://github.com/che-samples/bash/tree/devfilev2?che-editor=https://gist.githubusercontent.com/vinokurig/8240d845067c89e295e2cb4dc2c0070b/raw/ad3452ee49b0eeddfaf086f55557d46206073fea/che-theia.yaml
  1. Set some gitconfig settings e.g. user.name and user.email.
  2. Restart workspace.

See: The gitconfig settings are present.

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Happy Path Channel

HAPPY_PATH_CHANNEL=stable

@vinokurig
Copy link
Contributor Author

depends on eclipse-che/che#21147

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #1319 (c1aab10) into main (c299f59) will increase coverage by 3.95%.
The diff coverage is 39.52%.

@@            Coverage Diff             @@
##             main    #1319      +/-   ##
==========================================
+ Coverage   32.78%   36.74%   +3.95%     
==========================================
  Files         290      330      +40     
  Lines        9885    11393    +1508     
  Branches     1457     1572     +115     
==========================================
+ Hits         3241     4186     +945     
- Misses       6641     7202     +561     
- Partials        3        5       +2     
Impacted Files Coverage Δ
...theia-about/src/browser/about-che-theia-dialog.tsx 0.00% <0.00%> (ø)
...credentials/src/browser/che-credentials-service.ts 0.00% <0.00%> (ø)
...entials/src/browser/credentials-frontend-module.ts 0.00% <0.00%> (ø)
...eia-credentials/src/common/credentials-protocol.ts 0.00% <0.00%> (ø)
...eia-credentials/src/node/che-credentials-server.ts 0.00% <0.00%> (ø)
...s/src/node/che-theia-credentials-backend-module.ts 0.00% <0.00%> (ø)
...ashboard/src/browser/che-theia-dashboard-module.ts 0.00% <0.00%> (ø)
...ia-dashboard/src/browser/theia-dashboard-client.ts 0.00% <0.00%> (ø)
...rowser/src/browser/che-mini-browser-environment.ts 0.00% <0.00%> (ø)
...in-ext/src/browser/che-sidecar-file-system-main.ts 100.00% <ø> (ø)
... and 295 more

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 c58f5bd...c1aab10. Read the comment docs.

@svor svor mentioned this pull request Feb 21, 2022
12 tasks
Copy link
Contributor

@svor svor left a comment

Choose a reason for hiding this comment

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

LGTM

@vinokurig vinokurig force-pushed the che-21115 branch 2 times, most recently from 9f92e3f to 2f0286f Compare April 1, 2022 11:26
@l0rd
Copy link
Contributor

l0rd commented Apr 8, 2022

I have just started a workspace using this editor definition, and although the git user and email are set in Theia preferences, Theia still shows the warning in the status bar:

image

Note that theia git email and username preferences were set previously, in another workspace.

@l0rd
Copy link
Contributor

l0rd commented Apr 8, 2022

And git commit from the terminal doesn't seem to like:

image

@l0rd
Copy link
Contributor

l0rd commented Apr 8, 2022

After editing the preferences the warning in the status bar disappear. Even when I restart the workspace 👍
But the terminal is still unhappy.

image

@vinokurig
Copy link
Contributor Author

@l0rd
I've fixed all the issues above, could you please take a look?

@l0rd
Copy link
Contributor

l0rd commented Apr 15, 2022

Thanks @vinokurig I will test it later

@l0rd
Copy link
Contributor

l0rd commented Apr 21, 2022

I have tried this PR and there is a problem. Theia now creates a configmap with the gitconfig. But if a gitconfig ConfigMap already existed in the namespace we will end up with 2 ConfigMaps:

$ oc get cm -l controller.devfile.io/mount-to-devworkspace=true
NAME                  DATA   AGE
git-config            1      12d
workspace-gitconfig   1      74m

And then workspaces will fail to start.

@l0rd
Copy link
Contributor

l0rd commented Apr 21, 2022

As discussed a couple of weeks ago I think that Theia should not create or update the git configmap. It should instead:

  • At startup, if git user and email are NOT in Theia preferences yet, it should read them in /etc/gitconfig if it exists
  • At startup, if git user and email are already in Theia preferences, it should add them in file $PROJECT_SOURCE/.git/config (if they are not there yet) so that git config is available when using the command line too. Note that /etc/gitconfig/ may have different values but Theia preference have priority: theia preferences override global git config.
  • When user sets or updates git user/email in Theia preferences, the file $PROJECT_SOURCE/.git/config should be updated accordingly so that the git config is also available when using the command line.

@l0rd
Copy link
Contributor

l0rd commented Apr 25, 2022

Other acceptance criteria:

  • If local git config is updated (manually by the user), Theia preferences should not be changed.
  • If local git config is created, Theia preferences should not be changed.
  • if Theia preferences are updated and local git config matches old theia preferences then local git config should be updated too.
  • if Theia preferences are updated and local git config didn't match old theia preferences then local git config should NOT be updated.

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
@vinokurig
Copy link
Contributor Author

PR update:

  • When user gitconfig home/theia/.gitconfig is updated with a new value, local gitconfig <project directory>/.git/config is updated with the value automatically.
  • When an existed value is updated in user gitconfig:
    • If the old value is equal with one in local gitconfig, it is updated in the local gitconfig.
    • If the old value is NOT equal with one in local gitconfig, it is NOT updated in the local gitconfig.
  • When a value is deleted in user gitconfig:
    • If the value is equal with one in local gitconfig, it is deleted in the local gitconfig.
    • If the value is NOT equal with one in local gitconfig, it is NOT deleted in the local gitconfig.

@l0rd @svor Please take a look

@vinokurig
Copy link
Contributor Author

To test this PR run a factory:

https://github.com/che-samples/bash/tree/devfilev2?che-editor=https://gist.githubusercontent.com/vinokurig/8240d845067c89e295e2cb4dc2c0070b/raw/ad3452ee49b0eeddfaf086f55557d46206073fea/che-theia.yaml

Copy link
Contributor

@svor svor left a comment

Choose a reason for hiding this comment

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

LGTM
screenshot-nimbusweb me-2022 05 06-15_52_01

@svor svor changed the title chore: store gitconfig in a configmap chore: update local git config when user changes Che Theia git settings May 10, 2022
@l0rd
Copy link
Contributor

l0rd commented May 11, 2022

I have tried your Theia image as mentioned in this PR description. I don't understand why I am asked to configure my git config email when it's already configured:

image

Note that I had a theia preferences configmap:

kubectl cm workspace-preferences-configmap -o json | jq -r '.data[]' | jq .
{
  "git.user.email": "mario.loriedo@gmail.com",
  "git.user.name": "Mario"
}

@vinokurig
Copy link
Contributor Author

@l0rd @svor

I have tried your Theia image as mentioned in this PR description. I don't understand why I am asked to configure my git config email when it's already configured

I've fixed this bug, could you please try again?

@vinokurig
Copy link
Contributor Author

@l0rd @svor
There is another bug: panel is not updated if a new workspace started with existing user git preferences, working on it

@vinokurig
Copy link
Contributor Author

@l0rd @svor

There is another bug: panel is not updated if a new workspace started with existing user git preferences, working on it

The bug has been fixed, could you please take a look?

@svor
Copy link
Contributor

svor commented May 13, 2022

seems ok to me, thanks for updating the PR

@svor
Copy link
Contributor

svor commented May 23, 2022

@l0rd @azatsarynnyy we are going to merge this PR after 2 days.
If you have any concerns with that, please let us know

@svor svor merged commit 6655bed into main May 25, 2022
@svor svor deleted the che-21115 branch May 25, 2022 09:08
@che-bot che-bot added this to the 7.49 milestone May 25, 2022
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.

5 participants