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

IDEA-157915: Fix symbolic-ref in HEAD #424

Conversation

illabefat
Copy link

No description provided.

@illabefat illabefat closed this Jul 5, 2016
@illabefat illabefat reopened this Jul 13, 2016
@klikh
Copy link
Member

klikh commented Jul 14, 2016

Have you submitted a contributor license agreement? If not, please check your mail box: I've just sent a sign request there via DocuSign.

@illabefat
Copy link
Author

Yes, I've just signed it.

@klikh
Copy link
Member

klikh commented Jul 14, 2016

Received, thanks.

I see that you've at first closed this pull request right after creation, and then reopened it a week later. But Github doesn't show any new/rewritten commits here. You just wanted to test it more, or you closed it by accident?

@illabefat
Copy link
Author

Yes, I just wanted PR to be first reviewed by my colleagues, but accidentally created it in JetBrains repo instead of my fork


@NotNull
File getBranchFile(String path) {
return new File(FileUtil.toSystemDependentName(myMainDir.getCanonicalPath() + File.separatorChar + path));
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this should be changed to

  @NotNull
  File getBranchFile(@NotNull String fullBranchName) {
    return file(myMainDir.getPath() + slash(fullBranchName));
  }

(see other get*File methods and isBranchFile())

@klikh
Copy link
Member

klikh commented Jul 17, 2016

Merged the patch with a few mentioned corrections. Thanks a lot!

@klikh klikh closed this Jul 17, 2016
@klikh
Copy link
Member

klikh commented Jul 18, 2016

Ah. Together we missed the fact that the linked reference can reside in .git/packed-refs. Currently it leads to RepoStateException: Couldn't load file /Users/loki/temp/t/.git/refs/heads/master.
Seems that this fix can't be easily adjusted. Temporary reverted.

@illabefat
Copy link
Author

And how can this happen? symbolic-ref in origin?

@klikh
Copy link
Member

klikh commented Jul 18, 2016

git pack-refs --all makes everything packed, including refs/heads/master thus the code can't find .git/refs/heads/master written in .git/HEAD.

It seems that to solve the issue we'll have first to read all references to a map, both from .git/refs and .git/packed-refs, and then resolve them by following links inside the map.

@illabefat
Copy link
Author

Thank you, I'll try to do something with this.

@klikh
Copy link
Member

klikh commented Jul 18, 2016

Cool, thanks. I've written a test which reveals the issue.

package git4idea.repo

import com.intellij.openapi.util.io.FileUtil
import com.intellij.openapi.vfs.LocalFileSystem
import git4idea.branch.GitBranchUtil.stripRefsPrefix
import git4idea.test.GitExecutor.cd
import git4idea.test.GitExecutor.git
import git4idea.test.GitPlatformTest
import git4idea.test.GitTestUtil.makeCommit
import java.io.File

class GitRepositoryReaderNew2Test : GitPlatformTest() {

  fun `test packed symbolic ref`() {
    cd(myProjectPath)
    git("init")
    makeCommit("file.txt")
    git("pack-refs --all")

    // verify master is packed
    val gitDir = LocalFileSystem.getInstance().refreshAndFindFileByIoFile(File(myProjectPath, ".git"))!!
    val repoFiles = GitRepositoryFiles.getInstance(gitDir)

    assertFalse("setup failed: master is not packed", File(repoFiles.refsHeadsFile, "master").exists())
    assertTrue("setup failed: master is not packed",
               FileUtil.loadFile(repoFiles.packedRefsPath).lines().any { it.endsWith("refs/heads/master") })

    val repo = GitRepositoryImpl.getInstance(myProjectRoot, myProject, false)
    assertEquals("HEAD should be on master", "master", stripRefsPrefix(repo.currentBranchName!!))
    assertEquals("HEAD revision is incorrect", git("rev-parse HEAD"), repo.currentRevision)
    assertEquals("master revision is incorrect",
                 git("rev-parse master"),
                 repo.branches.getHash(repo.branches.findLocalBranch("master")!!)!!.asString())
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants