p4-fusion icon indicating copy to clipboard operation
p4-fusion copied to clipboard

issue with case-sensitivity

Open yukwunhang opened this issue 1 year ago • 8 comments

We were hunting down a missing file after the import:

The CL's action is "branch". Within the CL, all files have integrate as the action. Except one, which has the action "branch".

After the import, we found that the file that has "branch" as the action is missing from the git repo.

p4-fusion is a godsend by the way. Thanks for open-sourcing this tool!

version: 1.10

yukwunhang avatar Sep 12 '22 21:09 yukwunhang

It doesn't seem immediately apparent on why this file action type is being excluded. The only case where we have special behaviour in p4-fusion is when the file type itself is binary. We don't have any special conditions for branched files.

  1. Could you check if the file had any changes in it done as a part of this CL?

Perforce can add files to a CL without ever making any changes to it, and so sometimes the file would appear in p4 describe -s CL but it wouldn't appear in the Git repo generated by p4-fusion because Git will only record changes in the CL if the file contents have actually changed.

  1. Are you using the same user across p4-fusion and P4V?

twarit-waikar avatar Sep 15 '22 07:09 twarit-waikar

I think this has to do with case-sensitivity. I should mention that this is a Windows-only project.

We found another merge CL with the same issue, and upon investigation the root cause is more apparent. In the problematic merge CLs, the missing files have different casing in the depot path:

File Revision Action IIn Folder
foo.txt 4 integrate //depot/Branch-Name/sub/dir
missing.txt 1 branch //depot/branch-name/sub/dir

It would seem that during a merge, the casing of the depot paths of some files can be different from the "canonical" path. Perforce then mark these files with "branch" as the action.

While file paths are case-sensitive in the Perforce backend, this is invisible to the user on Windows. If you sync down //depot/Branch-Name/... both foo.txt and missing.txt would be sync'd. (EDIT: this is true only if the Perforce server is running in case-insensitive mode (-C1) -- https://portal.perforce.com/s/article/2577)

On the other end, p4-fusion's --path is case sensitive (or maybe it's because it's a Linux-only tool). My guess is that since I passed //depot/Branch-Name/... to --path, and because missing.txt is technically not under //depot/Branch-Name, p4-fusion would then skip this file.

(Note: git-p4 seems to handle this correctly, but maybe it's because I also used it on Windows.)

yukwunhang avatar Sep 16 '22 23:09 yukwunhang

This probably needs more research into how some of the Perforce APIs that we call into support case insensitivity. I think we can add a flag for it in p4-fusion but needs to be throughly tested

twarit-waikar avatar Sep 20 '22 08:09 twarit-waikar

Looks like this might need an extra bit of MapApi.SetCaseSensitivity in the client mapping.

groboclown avatar Nov 30 '22 22:11 groboclown

If the file in question is being returned as a part of the p4 describe output of a particular CL, it might get culled due to the first boolean filter here: https://github.com/salesforce/p4-fusion/blob/3ee482466464c18e6a635ff4f09cd75a2e1bfe0f/p4-fusion/commands/change_list.cc#L66

And this function is kind of stupid actually: https://github.com/salesforce/p4-fusion/blob/3ee482466464c18e6a635ff4f09cd75a2e1bfe0f/p4-fusion/p4_api.cc#L98 Doesn't seem to be case-sensitive and only checks if the depot path (provided as --path) is present char-for-char in the file's path.

However, MapApi.SetCaseSensitivity might still be needed somewhere as @groboclown recommends since the client specs might still want to match with such a file path that only differs in its case.

twarit-waikar avatar Dec 06 '22 10:12 twarit-waikar

It might occur that removing the call to IsFileUnderDepotPath() and instead just relying on the client spec to perform the culling is all that's needed: https://github.com/salesforce/p4-fusion/blob/3ee482466464c18e6a635ff4f09cd75a2e1bfe0f/p4-fusion/commands/change_list.cc#L67

But the GitAPI will still need to find a way to handle case-insensitivity

twarit-waikar avatar Dec 06 '22 10:12 twarit-waikar

We should be able to set core.ignorecase using this libgit2 function - https://libgit2.org/libgit2/#HEAD/group/repository/git_repository_config

twarit-waikar avatar Dec 06 '22 10:12 twarit-waikar

Further Perforce documentation on its nuances of case-sensitivity: https://portal.perforce.com/s/article/3081 It'd be great if p4-fusion had an arg for case sensitivity, or could extract this information from the Perforce server, ex. via p4 info?

marcleblanc2 avatar Dec 04 '23 21:12 marcleblanc2