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

issue with case-sensitivity #53

Open
yukwunhang opened this issue Sep 12, 2022 · 8 comments
Open

issue with case-sensitivity #53

yukwunhang opened this issue Sep 12, 2022 · 8 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@yukwunhang
Copy link

yukwunhang commented Sep 12, 2022

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

@twarit-waikar twarit-waikar added the bug Something isn't working label Sep 13, 2022
@twarit-waikar
Copy link
Contributor

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?

@yukwunhang
Copy link
Author

yukwunhang commented Sep 16, 2022

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 yukwunhang changed the title "Branched" files are not imported issue with case-sensitivity Sep 16, 2022
@twarit-waikar
Copy link
Contributor

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 twarit-waikar added the help wanted Extra attention is needed label Sep 20, 2022
@groboclown
Copy link
Contributor

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

@twarit-waikar
Copy link
Contributor

twarit-waikar commented Dec 6, 2022

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:

if (p4->IsFileUnderDepotPath(fileData.depotFile, depotPath)

And this function is kind of stupid actually:

return STDHelpers::Contains(fileRevision, depotPath.substr(0, depotPath.size() - 3)); // -3 to remove the trailing "..."

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
Copy link
Contributor

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:

&& p4->IsFileUnderClientSpec(fileData.depotFile)

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

@twarit-waikar
Copy link
Contributor

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

@marcleblanc2
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants