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

[git] update to latest dugite-extra, dugite-no-gpl #11782

Merged
merged 2 commits into from
Nov 2, 2022

Conversation

marcdumais-work
Copy link
Contributor

@marcdumais-work marcdumais-work commented Oct 17, 2022

What it does

This PR updates @theia/git to consume the latest dugite-extra, which in turn uses the latest, recently uplifted dugite-no-gpl. The later is a fork of upstream dugite (minus GPL-licensed git binaries added at install time), that had not been updated in nearly 3 years.

Update: investigating a @theia/git unit test failure (see below), I noticed we needed to update @theia/git -> git-model -> GitError enum to match dugite-no-gpl, that had evolved since the last uplift. See commit message for more details.

How to test

Make sure the unit tests pass. Some manual testing of theia git extension would probably be a good idea to make sure nothing was broken.

Review checklist

Reminder for reviewers

@marcdumais-work marcdumais-work changed the title [git] update to latest dugite-extra, dugite-no-gpl [WIP] [git] update to latest dugite-extra, dugite-no-gpl Oct 17, 2022
@marcdumais-work marcdumais-work added the git issues related to git label Oct 17, 2022
@marcdumais-work
Copy link
Contributor Author

We have a unit test failure in @theia/git, that I will address by addding a commit:

 @theia/git:   1) git
@theia/git:        ls-files
@theia/git:          errorUnmatched - ../outside.txt should not exist:
@theia/git:      Uncaught GitError: This path is not a valid path inside the repository.
@theia/git:       at new GitError (D:\a\theia\theia\node_modules\dugite-extra\lib\core\git.js:91:28)
@theia/git:       at D:\a\theia\theia\node_modules\dugite-extra\lib\core\git.js:266:27
@theia/git:       at step (D:\a\theia\theia\node_modules\dugite-extra\lib\core\git.js:59:23)
@theia/git:       at Object.next (D:\a\theia\theia\node_modules\dugite-extra\lib\core\git.js:40:53)
@theia/git:       at fulfilled (D:\a\theia\theia\node_modules\dugite-extra\lib\core\git.js:31:58)
@theia/git:       at processTicksAndRejections (node:internal/process/task_queues:96:5)
@theia/git: error Command failed with exit code 1.
@theia/git: info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
lerna ERR! yarn run test exited 1 in '@theia/git'
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
lerna WARN complete Waiting for 1 child process to exit. CTRL-C to exit immediately.
error Command failed with exit code 1.
Error: Process completed with exit code 1.

@marcdumais-work marcdumais-work changed the title [WIP] [git] update to latest dugite-extra, dugite-no-gpl [git] update to latest dugite-extra, dugite-no-gpl Oct 17, 2022
@marcdumais-work
Copy link
Contributor Author

cc: @thegecko : This is the PR that introduces the uplifted dugite-no-gpl, that I mentioned recently. Can someone in your team help review and test?

@thegecko
Copy link
Member

cc @arekzaluski @mcgordonite could you check this please?

This commit updates @theia/git to consume the latest `dugite-extra`,
which in turn uses the latest, recently uplifted `dugite-no-gpl`.

Signed-off-by: Marc Dumais <[email protected]>
Upstream dugite has changed its GitError enum [1], which became part
of `dugite-no-gpl` after its uplift. @theia/git has it own GitError enum [2],
that I believe needs to be aligned with `dugite's`.

This commit atttempts to align our version of that enum. After this update,
the @theia/git unit tests now pass.

[1] https://github.com/theia-ide/dugite/blob/main/lib/errors.ts#L2-L63
   starting at index 16 a new item was inserted, offsetting from that point.
   New entries were also added towards the end, offsetting the GitHub errors.
   finally new entries were added after the GitHub section.
[2] https://github.com/eclipse-theia/theia/blob/86fe42854196b1c3cea90b95840acae3e0f00061/packages/git/src/common/git-model.ts#L421-L467

Signed-off-by: Marc Dumais <[email protected]>
@marcdumais-work
Copy link
Contributor Author

@vince-fugnitto I have rebased this PR on the latest master - thanks for volunteering to review it!

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I confirmed the following:

  • git decorations work as expected (added, modified, deleted)
  • stage works as expected
  • stage all works as expected
  • unstage works as expected
  • unstage all works as expected
  • discard works as expected
  • discard all works as expected
  • toggling tree vs list works as expected
  • correct git status in the statubar
  • git diff works as expected
  • switching repository in a multi-root workspace works as expected
  • confirmed creating a commit, sign-off and pushing works as expected

@marcdumais-work marcdumais-work merged commit d109bc9 into master Nov 2, 2022
@marcdumais-work marcdumais-work deleted the dugite-extra-update branch November 2, 2022 15:51
@github-actions github-actions bot added this to the 1.32.0 milestone Nov 2, 2022
@vince-fugnitto vince-fugnitto added the dependencies pull requests that update a dependency file label Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file git issues related to git
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants