Allow amending of the initial commit in the repository #5451

westbury · 2019-06-13T14:13:32Z

This fixes #4972

This fix is not a high priority as the problem has always existed. However I thought I would submit the fix so that we don't keep finding the problem or wondering if this is the cause of other issues.

To test this, use a repository with few commits and amend back to the initial commit. One should not be able to amend the initial commit.

svenefftinge · 2019-06-13T14:27:54Z

Would be great to be able to amend the initial commit. Not sure how, but maybe we can improve the git reset operation somehow?

westbury · 2019-06-14T23:42:57Z

Would be great to be able to amend the initial commit. Not sure how, but maybe we can improve the git reset operation somehow?

This may be easier than I thought it would be. On further thought, I would not change git reset. The problem with that is that git reset would leave the repository in a state in which there is no HEAD and this could be confusing to other bits of the code that call git reset. Also git reset could be called with all sorts of different options and it would not be possible or desirable to execute git reset with all those options in any kind of consistent way while converting the repository into an empty, no HEAD, state. It's better to keep all this no-HEAD weirdness to the one place that needs it, the GitAmendSupport class.

Don't review this PR yet. After #5484 is merged I'll amend this PR. If the initial commit is amendable then we don't need the amendable flag.

westbury · 2019-07-14T22:02:43Z

I have now re-written this to allow the amending of the initial commit. There is no need to ever disable the amend button because once the initial commit has been amended there is no head commit and hence no 'amend' button.

There are no Git commands that I could find to reset a repository to before the initial commit. However one can get back to the same state (the state immediately after 'git init') by deleting refs/heads/master. Actually it reads HEAD to find the branch as the user could have changed the branch from master.

There is no SHA that represents the state before the first commit, so that state is represented in this PR by undefined ScmCommit or an id of undefined. This required some changes to support an undefined commit. Note that the git log command goes all the way back to the initial commit if fromRevision is not defined, which is exactly what we want. This ensures that the amending commits is restored correctly after a refresh (test by amending commits back to and including the initial commit, then refresh).

westbury · 2019-09-12T09:57:41Z

@kittaakos I was wondering if you had time to have a look at this one. It's left over from the work on the Git/SCM/VsCode switchover a few months ago.

kittaakos · 2019-09-12T11:08:18Z

@westbury, thanks for the reminder. I am a little bit out of the Theia loop recently, but I will find some time to get this done. Also, I owe you a review for the output logging for Electron 😊

kittaakos · 2019-09-17T11:51:28Z

I have tried it with a single repository; I got a strange result when I have refreshed the browser.

Steps to reproduce:

Open an empty workspace.
Clone a Git repository.
Amend a few commits. Check how many commits you see.
Refresh the browser.
Check how many commits now you can see. It's more than expected.

westbury · 2019-09-22T20:49:29Z

I have tried it with a single repository; I got a strange result when I have refreshed the browser.

@kittaakos thank you for testing and for finding the problem. The problem you have found is caused by commits that have multiple parents, as you are amending merge commits. When you press 'amend', it picks the first parent so you see a linear list of commits. However when you refresh you are seeing all the commits on all paths from the new head to the original head.

This does need to be fixed, and I will come up with a fix. However this problem is separate from this issue which covers the handling of the initial commit. The problem occurs in master so perhaps should be covered by a separate PR.

svenefftinge · 2019-09-23T06:11:05Z

packages/git/src/browser/git-scm-provider.ts

@@ -413,7 +416,23 @@ export class GitAmendSupport implements ScmAmendSupport {
    }

    public async reset(commit: string): Promise<void> {
-        await this.git.exec(this.repository, ['reset', commit, '--soft']);
+        if (commit === 'HEAD~' && await this.isHeadInitialCommit()) {


In VS Code they seem to be able to handle this without using the filesystem.
https://github.com/microsoft/vscode/blob/master/extensions/git/src/commands.ts#L1464

In VS Code they seem to be able to handle this without using the filesystem

@svenefftinge you are totally correct. git update-ref -d HEAD does the trick perfectly. Thanks for pointing that out. It tests fine. I have amended the commit accordingly.

Signed-off-by: Nigel Westbury <nigelipse@miegel.org>

westbury · 2019-10-11T16:03:15Z

@kittaakos Are you able to re-verify this PR? The last change was to replace the use of file-system with the git update-ref command.

kittaakos

I have verified the change-set in Gitpod, it worked. I could amend to the initial commit of the repo.

akosyakov requested review from akosyakov and kittaakos June 13, 2019 14:16

kittaakos mentioned this pull request Jun 13, 2019

Request 'log' failed from scm amend #5435

Closed

westbury mentioned this pull request Jun 14, 2019

Improve support for empty Git repository in Git view #5484

Merged

westbury force-pushed the GH4972 branch from 75fc2f3 to 35d0a26 Compare July 14, 2019 21:41

westbury changed the title ~~Disable 'Amend' button when on the initial commit in the repository~~ Allow amending of the initial commit in the repository Jul 14, 2019

westbury force-pushed the GH4972 branch from 35d0a26 to e7723a3 Compare September 11, 2019 22:31

akosyakov requested a review from svenefftinge September 12, 2019 10:14

akosyakov added git issues related to git scm issues related to the source control manager labels Sep 12, 2019

westbury mentioned this pull request Sep 22, 2019

Show in amending commit list only the first parents #6242

Merged

svenefftinge reviewed Sep 23, 2019

View reviewed changes

westbury force-pushed the GH4972 branch 2 times, most recently from cfec8f5 to 5322e24 Compare September 23, 2019 22:29

Support amending of initial commit

85cb4a6

Signed-off-by: Nigel Westbury <nigelipse@miegel.org>

westbury force-pushed the GH4972 branch from 5322e24 to 85cb4a6 Compare September 23, 2019 22:37

westbury requested a review from svenefftinge September 30, 2019 17:59

kittaakos approved these changes Oct 15, 2019

View reviewed changes

westbury merged commit 1a8188f into eclipse-theia:master Oct 17, 2019

westbury deleted the GH4972 branch October 17, 2019 08:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow amending of the initial commit in the repository #5451

Allow amending of the initial commit in the repository #5451

westbury commented Jun 13, 2019

svenefftinge commented Jun 13, 2019

westbury commented Jun 14, 2019

westbury commented Jul 14, 2019

westbury commented Sep 12, 2019

kittaakos commented Sep 12, 2019

kittaakos commented Sep 17, 2019

westbury commented Sep 22, 2019

svenefftinge Sep 23, 2019

westbury Sep 23, 2019

westbury commented Oct 11, 2019

kittaakos left a comment

Allow amending of the initial commit in the repository #5451

Allow amending of the initial commit in the repository #5451

Conversation

westbury commented Jun 13, 2019

svenefftinge commented Jun 13, 2019

westbury commented Jun 14, 2019

westbury commented Jul 14, 2019

westbury commented Sep 12, 2019

kittaakos commented Sep 12, 2019

kittaakos commented Sep 17, 2019

westbury commented Sep 22, 2019

svenefftinge Sep 23, 2019

Choose a reason for hiding this comment

westbury Sep 23, 2019

Choose a reason for hiding this comment

westbury commented Oct 11, 2019

kittaakos left a comment

Choose a reason for hiding this comment