Hi there,

After running a sync for a large project I found this error:

[ ERROR @ AddFileToIndex:147 ] GitAPI: -1:10: invalid path: 'redacted/redacted/redacted/redacted/redacted/redacted/redacted/redacted/Private/Interfaces/.git'

It seems that p4-fusion is trying to add .git from a submodule located in a deeper directory.

By looking at p4-fusion source code (change_list.cc#L57), p4-fusion is ignoring .git/*, but not the .git per se. I believe this is a bug and it should also ignore files ending with .git.

Does it make sense? After implementing this change locally, the project was successfully synced.

We use stream branches in our development. I created a virtual branch to narrow down the scope of what I want to export to git. I ran in to a number of issues, which I would normally open one for each, but you may need the whole story for them to all make sense. So sorry for the long story / issue.

Issue 1 - Mappings don't work properly

So I ran something like this after creating my new virtual branch:

p4-fusion --client "eyen_p4_fusion" \
    --includeBinaries true  \
    --path "//origin/main/..." \
    --port "perforce.theobjects.com:1666" \
    --printBatch 10 \
    --src /work/git \
    --user user \
    --lookAhead 100

but then I get an error like this:

[ PRINT @ Main:31 ] Running p4-fusion from: /work/p4-fusion/build/p4-fusion/p4-fusion
[ SUCCESS @ InitializeLibraries:144 ] Initialized P4Libraries successfully
[ PRINT @ Main:80 ] Updated client workspace view eyen_p4_fusion with 26 mappings
[ ERROR @ Main:92 ] The depot path specified is not under the p4_fusion_test client spec. Consider changing the client spec so that it does. Exiting.

I know it's under the spec. I just created the virtual branch and workspace.

So I just removed the return statement here and then I got further.

But before going further, I tracked down the problem to this: According to the docs

 Translate() is designed to map single files. 
To model the effect of passing a broader path through a mapping, create a new one-sided mapping that represents that path and Join() it with the other mapping.

In my workspace mapping, I had this:

//origin/main/parent_folder/folder1 /work/space/folder1/...
//origin/main/parent_folder/folder2 /work/space/folder2/...

But then when I try to map the --path argument //origin/main/..., Translate() is not able to determine that the path is valid.

So that's the crux of the issue and I don't know enough of the perforce api to know how to check properly. I can ask them for support, but I suppose you are more knowledgeable than I am.

Issue 2 - It crashes with malloc(): unaligned tcache chunk detected

After removing that return statement, I then get a crash here: These are the parameters printed at the beginning

[ PRINT @ Main:134 ] Perforce Port: ---                                                                  
[ PRINT @ Main:135 ] Perforce User: --                                                                                                                 
[ PRINT @ Main:136 ] Perforce Client: ---                                                                                                     
[ PRINT @ Main:137 ] Depot Path: //origin/main/...                                                                                                       
[ PRINT @ Main:138 ] Network Threads: 48                                                                                                                     
[ PRINT @ Main:139 ] Print Batch: 10                                                                                                                         
[ PRINT @ Main:140 ] Look Ahead: 100                                                                                                                         
[ PRINT @ Main:141 ] Max Retries: 10                                                                                                                         
[ PRINT @ Main:142 ] Max Changes: -1                                                                                                                         
[ PRINT @ Main:143 ] Refresh Threshold: 100                                                                                                                  
[ PRINT @ Main:144 ] Fsync Enable: 0                                                                                                                         
[ PRINT @ Main:145 ] Include Binaries: 1                                                                                                                     
[ PRINT @ Main:146 ] Profiling: 0                                                                                                                            
[ PRINT @ Main:147 ] Profiling Flush Rate: 1000                             

I didn't know what the tcache bug meant so I ran it with ASAN and the code runs now. I don't know what this bug means. Maybe I should force it to use less network threads, but it ran.

So now I have my git repo, but this brings me to issue 3.

Issue 3: Workspace mappings are not respected

In my workspace mappings, I have mappings like this:

//origin/main/parent_folder/folder2 /work/space/folder2/...

but in my git repo, what I see when I do clone is this: git_folder/parent_folder/folder2 instead of git_folder/folder

Ideally, I want the p4-fusion to Trasnlate() the mapping to the local folder and then strip the workspace root and then append the stripped path to the git repo. I don't know where to go for there.

Receiving the following error when trying p4-fusion out

[ WARNING @ Run:126 ] Retrying: p4 changes -l -s submitted -r //src/... [ ERROR @ HandleError:13 ] Received error: error Usage: changes [-i -t -l -L -f -c client -m count -s status -u user] [files...] Invalid option: -r.

p4 changes has no '-r' option, although the docs state it does.

$ p4 -V Perforce - The Fast Software Configuration Management System. Copyright 1995-2022 Perforce Software. All rights reserved. This product includes software developed by the OpenSSL Project for use in the OpenSSL Toolkit (http://www.openssl.org/) Version of OpenSSL Libraries: OpenSSL 1.1.1n 15 Mar 2022 See 'p4 help [ -l ] legal' for additional license information on these licenses and others. Extensions/scripting support built-in. Parallel sync threading built-in. Rev. P4/LINUX26X86_64/2022.1/2305383 (2022/06/28).

Hi,

Thanks for open sourcing this tool. It's definitely very fast when compared to git-p4.py

Do you plan to add the sync command ?

I am willing contribute its implementation if you can give me some pointers on how you'd like it done.

Thanks, Mathieu.

In rare cases, p4-fusion crashes when running ThreadPool::ShutDown(), where the P4API destructor (defined in the Helix Core C++ API library) runs into a segmentation fault.

Version: 1.9 OS: CentOS 7

Crash backtrace from gdb:

Program terminated with signal 11, Segmentation fault.
#0  Signaler::DeleteOnIntr (this=0x55a7a6720460 <signaler>, ptr=0x7f1b1893c3b0) at ../p4/sys/signaler.cc:229
229	../p4/sys/signaler.cc: No such file or directory.
(gdb)
(gdb) where
#0  Signaler::DeleteOnIntr (this=0x55a7a6720460 <signaler>, ptr=0x7f1b1893c3b0) at ../p4/sys/signaler.cc:229
#1  0x000055a7a62c4b13 in Rpc::~Rpc (this=0x7f1b1893c3b0, __in_chrg=<optimized out>) at ../p4/rpc/rpc.cc:312
#2  0x000055a7a62a3ef7 in Client::~Client (this=0x7f1b1893c3b0, __in_chrg=<optimized out>) at ../p4/support/strbuf.h:369
#3  0x000055a7a629c8ac in ClientApi::~ClientApi (this=0x7f1b188df100, __in_chrg=<optimized out>) at ../p4/client/clientapi.cc:21
#4  0x000055a7a6298d4c in _Destroy<P4API> (__pointer=0x7f1b188df100) at /usr/include/c++/10.3.1/bits/stl_construct.h:135
#5  __destroy<P4API*> (__last=<optimized out>, __first=0x7f1b188df100) at /usr/include/c++/10.3.1/bits/stl_construct.h:152
#6  _Destroy<P4API*> (__last=<optimized out>, __first=<optimized out>) at /usr/include/c++/10.3.1/bits/stl_construct.h:185
#7  _Destroy<P4API*, P4API> (__last=<optimized out>, __first=<optimized out>) at /usr/include/c++/10.3.1/bits/alloc_traits.h:738
#8  _M_erase_at_end (__pos=0x7f1b188df100, this=0x55a7a671f750 <ThreadPool::GetSingleton()::singleton+112>) at /usr/include/c++/10.3.1/bits/stl_vector.h:1796
#9  clear (this=0x55a7a671f750 <ThreadPool::GetSingleton()::singleton+112>) at /usr/include/c++/10.3.1/bits/stl_vector.h:1499
#10 ThreadPool::ShutDown (this=0x55a7a671f6e0 <ThreadPool::GetSingleton()::singleton>) at /tmp/tmp.CjOLdF/p4-fusion-src/p4-fusion/thread_pool.cc:78
#11 0x000055a7a6299385 in ThreadPool::ShutDown (this=<optimized out>) at /tmp/tmp.CjOLdF/p4-fusion-src/p4-fusion/thread_pool.cc:58
#12 0x000055a7a6289fc1 in Main(int, char**) () at /tmp/tmp.CjOLdF/p4-fusion-src/p4-fusion/main.cc:308
#13 0x000055a7a627b027 in main () at /tmp/tmp.CjOLdF/p4-fusion-src/p4-fusion/main.cc:344
#14 0x00007f1b18d31a03 in ?? ()
#15 0x00007f1b18d319dc in ?? ()
#16 0x00007ffc0b6fce20 in ?? ()
#17 0x0000000000000000 in ?? ()

I was downloading this repo on a slow connection and wondered why it took so long.

$ du -hs vendor/libgit2/tests/
53M	vendor/libgit2/tests/

Minor issue, feel free to close out and ignore.

You can use OpenMP for multi-threaded calculations

It is supported by most C ++ compilers (GCC, Clang and MSVC), this easily applies to a for loop for example, You can even use another machine or the graphics card to perform some part of the calculations.

More info: https://bisqwit.iki.fi/story/howto/openmp/ https://medium.com/swlh/introduction-to-the-openmp-with-c-and-some-integrals-approximation-a7f03e9ebb65

Exemple my brute-force project: https://github.com/bensuperpc/GTA_SA_cheat_finder/blob/196dcedf30e81317148d025961b65f7213aa47e1/source/main.cpp#L89

This looks like something that might have caused issues but surprisingly hasn’t so far, which means p4-fusion is not corrupting the repo even if it gets interrupted. However, we should be on the safe side and also add the graceful exit handler for SIG_KILL in addition to SIG_INT (SIG_INT handler is already implemented)

p4-fusion doesn't have a way to know the version of the built binary. We should fix this so that it is easier for people to work with multiple versions or people making p4-fusion version upgrades in their workflows

The number of retries when testing the connection before actually carrying out the usual business is hard-coded to 5.

It doesn't seem to me that users would require flexibility in a simple connection test.

Fixes #41

I decided to implement it as a runtime switch instead of a compilation flag so that the same binary is configurable, and doesn't require a re-compilation to only disable colored outputs.

Adds branch support for ticket #54. This is a fairly big rewrite, and my C++ skills are pretty rusty, so it should have some eyes put on it.

Notes on the code changes:

• The fundamental changes to the code are at the change_list.cc level. Code now is filtered through the branch_set.cc code, and that's also where the files are split into branch groups (a collection of files that share a source + target branch for a single changelist).
• The FileData structure how has more responsibility, and some of the file inspection code has been moved into it (which pushes Perforce specific knowledge out of P4API, but it seems contained better this way). In order to be safe with memory, since it can potentially be very heavy weight, it's a wrapper around another structure through a shared pointer. This makes copy constructor semantics more forgiving. It has some weirdness, though, due to it not being a const only structure, which means a lot of invocations that read it might also have side effects on the object. Not very pretty.
• The inspection into file source comes from filelog_result.cc. It's a bit slower, so non-branch runs still use describe. This implementation works with the arguments in P4API to have the change history for only the first revision of each file in the changelist.

A few things that are dangling and not completed with this PR:

• Documentation. There's new options and behaviors that need docs. An initial pass at some documentation in the readme is present. Once this PR gets ironed out, then I can revisit them.
• Version number bump. I don't know if this is significant to take a major version bump. It should behave the same if you use the original arguments, so maybe just a minor bump?
• The commit description in git_api.cc could be enhanced, especially around the depot path.
• The fstat code was added to have a merge be against an older revision of a branch, but that logic is extremely tricky even if done right, and there's so many ways it can go wrong (see my comments in #54). This should be removed. I'll probably do that shortly. Done
• There's still some duplicate code in various places that could be brought together into a single call, especially in the git_api.

As Perforce supports automatic labels, which are a view mapping + a revision marker, it's possible to map these to a Git tag under very limited criteria:

1. The Label contains the field Revision with a value of @(integer)
2. The Label contains a View with a path that fully contains the depot path being copied.
3. The (integer) revision is a changelist that the transfer includes.

Because this label fully matches to a Git commit, a corresponding tag can be created off of the label.

The error seems to only occur in the musl library, and doesn't mention any p4-fusion specific functions in the stacktrace, even in a debug build. However, we only see this in alpine builds.

p4-fusion crashes in this case either at the end of the execution, or before starting the Git commit process (or when the tool is downloading files)

GDB core dump below:

Core was generated by `p4-fusion --path //depot/path/... --client p4-client'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007f5bd160c896 in ?? () from /lib/ld-musl-x86_64.so.1
[Current thread is 1 (LWP 296)]
(gdb) where
#0  0x00007f5bd160c896 in ?? () from /lib/ld-musl-x86_64.so.1
#1  0x00007f5bd160cc2f in ?? () from /lib/ld-musl-x86_64.so.1
#2  0x0000000000000000 in ?? ()

Could you add date/time to the progress messages? Messages like this one:

CL 1218143 --> Commit 4d3be6af2d1251f83d9462dc1bc34ed3ab34764c with 6 files (23515/111346|499). Elapsed 59.9115 mins. 223.776 mins left.

[https://github.com/salesforce/p4-fusion/blob/master/p4-fusion/main.cc#L301]

The docs show that this only works for single branches and even then, only branches containing single paths. Perforce allows branches to be located in any location, and even have different mappings for different branches.

Proposal: Allow the specification of a configuration file that describes the branches and the views used by them to sync:

[main]
//depot/main/config/my-component/... config/...
//depot/main/pkg/my-subcomponent-a/... pkg/my-subcomponent-a/...
//depot/main/pkg/my-subcomponent-b/... pkg/my-subcomponent-b/...

[dev]
//depot/comp/my-component/branches/dev/... ...

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