libgit2 / libgit2sharp

Git + .NET = ❤
http://libgit2.github.com
MIT License
3.15k stars 887 forks source link

The set filemode is incorrect #1953

Open georgii-sosnovikov opened 2 years ago

georgii-sosnovikov commented 2 years ago

Reproduction steps

Azure DevOps

  1. Create YAML Pipeline just for monitoring filemode (show all files with filemode, script: git ls-files --stage) and create build validation policy for main branch

  2. Create file "test1.sh" in folder "folder1", create PR to main and complete it

  3. On new branch make changes, push it, hard reset to previous commit, create "folder2", move "test1.sh" to this folder, change filemode to 100755 (git update-index --chmod=+x file_name), force push, create PR to main and complete it

  4. Switch to previous commit (before changing folder and file mode), push any changes, create PR to main and check build validation pipeline logs on the PR

Expected behavior

File mode is 100755 (new)

Actual behavior

File mode is 100644 (old)

Version of LibGit2Sharp (release number or SHA1)

14.0.29222.1

Operating system(s) tested; .NET runtime tested

Windows 10, .NET 4.8

ethomson commented 2 years ago

I'm not sure exactly what you mean by these actions:

On new branch make changes, push it, hard reset to previous commit

...and...

Switch to previous commit (before changing folder and file mode), push any changes,

What LibGit2Sharp methods are you invoking to do this work? Can you share sample code that reproduces this?

Also:

14.0.29222.1

is not a LibGit2Sharp version. Can you be more precise about the version of LibGit2Sharp that you're using?

georgii-sosnovikov commented 2 years ago

I found simpler reproduction steps:

Azure DevOps:

  1. Create branch dev1 from main, create file1.sh in folder1, merge to main
  2. Create branch test based on main
  3. Create branch dev2 from main, move file1.sh to folder2 and change mode for file1.sh, merge to main
  4. Checkout to test branch, push any changes.
  5. Merge test branch to main branch (OK, file1.sh 100755 mode)
  6. Check mode for file1.sh

Expected: file1.sh 100755 mode Actual: file1.sh 100644 mode

When doing step 5 using git bash, the problem does not occur.

Method that returns the wrong file mode - MergeCommitsIntoIndex with the following mergeOptions: FailOnConflict - false, FindRenames - true, MergeFileFlavor - Normal, RenameThreshold - 100, SkipReuc - false, TargetLimit - 500

LibGit2Sharp version - 0.26

georgii-sosnovikov commented 2 years ago

@ethomson Gentle ping, it is still problem for us

jrb-github commented 1 year ago

I added a thumbs-up on the previous comment, but I'll add another gentle ping as I suspect that doesn't bump the thread.

georgii-sosnovikov commented 5 months ago

@ethomson , could you please look into this?

ethomson commented 5 months ago

I'm sorry, I don't understand what the problem is. Can you show me some .NET code?

Are you reporting a bug in Azure DevOps?

ethomson commented 5 months ago

Err.. @georgii-sosnovikov — I see that you're part of the Microsoft org in GitHub. Are you on the Azure DevOps team?

georgii-sosnovikov commented 5 months ago

Hi @ethomson , yes, I am.

Let me please add a bit of context here. When merging with pull requests, the MergeCommitsIntoIndex method from LibGit2Sharp is used and in one of the cases we found, there is some unexpected behavior

The 1st case (where we see the bug)

The source branch, let's call it "test", contains:

The target branch to merge into is called "main" and contains:

When the PR is created (test -> main), you can see that only the README.md file is actually modified (the comparison is done through the base commit of both branches) and file1.sh should not be changed. But during the PR merge, the MergeCommitsIntoIndex method returns folder2/file1.sh with a NonExecutableFile mode 100644, when it should remain as 100755 in main, no changes were made to this file in this PR.

image

The passed mergeOptions you can see in my comment above

The 2nd case (everything is fine here)

The source branch, let's call it "test", contains

The target branch to merge into is called "main" and contains:

In this case, the only change the PR (test -> main) introduces is a change to the README.md file (just like in the first case) and file1.sh file should not be changed during merge and in fact when the PR is finished, file1.sh file is not changed. The MergeCommitsIntoIndex method returns the expected result and everything after that works correctly.

I hope it's going to shed some light on the issue